Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JS declarations for an export assigned class expression can conflict with local names #33626

Closed
weswigham opened this issue Sep 26, 2019 · 11 comments · Fixed by #34786
Closed
Assignees
Labels
Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files

Comments

@weswigham
Copy link
Member

weswigham commented Sep 26, 2019

Per this comment on the PR, the JS declaration emitter currently has a known issue where an export assigned class expression with a name can cause a conflict with another named declaration in the file and produce an invalid declaration file.

So, for example, this:

class A { member = new Q(); };
class Q { x = 12 };
module.exports = class Q { x = new A(); }

currently emits along the lines of

declare class A { member: Q; };
declare class Q { x: number };
declare class Q { x: A; }
exports = Q;

(because it reused the anonymous class's name for the hoisted class declaration) which isn't quite right.

@weswigham weswigham added Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files labels Sep 26, 2019
@Bnaya
Copy link

Bnaya commented Oct 11, 2019

I have a similar issue with esm default export:
code:

import React from "react";
/**
 * @type {React.FunctionComponent}
 */
const TabbedShowLayout = ({
    classes: classesOverride,
    className,
    version,
}) => {
    return (
        <div className={className} key={version}>
            {className}
        </div>
    );
};


TabbedShowLayout.defaultProps = {
    tabs: "default value"
};

export default TabbedShowLayout;

emitted declarations:

export default TabbedShowLayout;
/**
 * @type {React.FunctionComponent}
 */
declare const TabbedShowLayout: React.FunctionComponent;
declare namespace TabbedShowLayout {
    export const defaultProps: Partial<any> | undefined;
}
import React from "react";

Errors:
Cannot redeclare block-scoped variable 'TabbedShowLayout'.

@Bnaya
Copy link

Bnaya commented Oct 11, 2019

Additional breakage with javascript:

import React from "react";
import PropTypes from "prop-types"

const TabbedShowLayout = ({
}) => {
    return (
        <div />
    );
};

TabbedShowLayout.propTypes = {
    version: PropTypes.number,

};

TabbedShowLayout.defaultProps = {
    tabs: undefined
};

export default TabbedShowLayout;

d.ts:

export default TabbedShowLayout;
declare function TabbedShowLayout({}: {}): JSX.Element;
declare namespace TabbedShowLayout {
    export namespace propTypes {
        import version = PropTypes.number;
        export { version };
    }
    export namespace defaultProps {
// Cannot export 'undefined'. Only local declarations can be exported from a module
        export { undefined as tabs };
    }
// Import declarations in a namespace cannot reference a module.ts(1147)
// Cannot find module 'prop-types'.ts(2307)
    import PropTypes from "prop-types";
}

@Bnaya
Copy link

Bnaya commented Oct 11, 2019

I'm not sure how common is it to emit invalid types
But i think it will be very useful if the emitter would check the declarations it emits as a fail safe,
and warn the user what something went wrong. maybe prompt to open an issue or so.

@Bnaya
Copy link

Bnaya commented Oct 15, 2019

Is it on the roadmap to 3.7 final?

@weswigham
Copy link
Member Author

It should be, yeah - I'm just on vacation this week, so probably won't progress on it for a bit.

@Bnaya
Copy link

Bnaya commented Oct 25, 2019

I hope this issue won't be forgotten for 3.7 🙏😰

@weswigham
Copy link
Member Author

@Bnaya I have a build over in #34786 that I'd love if you could give a try.

@Bnaya
Copy link

Bnaya commented Oct 29, 2019

Thank you very much!
seems like most of the issues are gone

some of the findings:

import React from 'react';
import PropTypes from 'prop-types';

function Tree({ allowDropOnRoot }) {
  return <div />
}

Tree.propTypes = {
    classes: PropTypes.object,
};

Tree.defaultProps = {
    classes: {},
    parentSource: 'parent_id',
};

export default Tree;

emits broken:

export default Tree;
declare function Tree({ allowDropOnRoot }: {
    allowDropOnRoot: any;
}): JSX.Element;
declare namespace Tree {
    export namespace propTypes {
        export const classes: PropTypes.Requireable<object>;
    }
    export namespace defaultProps {
 // Cannot find name 'classes_1'
        export { classes_1 as classes };
        export const parentSource: string;
    }
}
import PropTypes from "prop-types";
//# sourceMappingURL=Tree.d.ts.map

When removing the propTypes.classes the problem is gone.

EDIT:
Also it depends on the namespace for the prop-types module and not importing it,
even that on the source, its importing it

@weswigham
Copy link
Member Author

@Bnaya there should be another build over in #34786 that also fixes that.

@Bnaya
Copy link

Bnaya commented Oct 30, 2019

Works!

Thank you!

@Bnaya
Copy link

Bnaya commented Nov 8, 2019

Probably a related issue:
#34994

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants