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: Some more improvements to d.ts file analysis #9457

Merged
merged 8 commits into from
Jun 20, 2022

Conversation

asgerf
Copy link
Contributor

@asgerf asgerf commented Jun 8, 2022

Some more improvements needed for model generation, mainly by improving analysis of .d.ts files.

Evaluation looks OK

@asgerf asgerf added the JS label Jun 8, 2022
@asgerf asgerf force-pushed the js/madman-prep2 branch 4 times, most recently from f09eac8 to d36e619 Compare June 14, 2022 08:15
@asgerf asgerf added no-change-note-required This PR does not need a change note Awaiting evaluation Do not merge yet, this PR is waiting for an evaluation to finish labels Jun 14, 2022
@asgerf asgerf force-pushed the js/madman-prep2 branch 2 times, most recently from 7815c49 to 437391b Compare June 16, 2022 09:50
@asgerf asgerf force-pushed the js/madman-prep2 branch from 437391b to 6a4b3a1 Compare June 17, 2022 12:40
@asgerf asgerf removed the Awaiting evaluation Do not merge yet, this PR is waiting for an evaluation to finish label Jun 17, 2022
@asgerf asgerf marked this pull request as ready for review June 17, 2022 12:43
@asgerf asgerf requested a review from a team as a code owner June 17, 2022 12:43
Copy link
Contributor

@erik-krogh erik-krogh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very quick first look.
Seems reasonable, but two comments.

return new ExternalModuleDeclaration(loc, (Literal) nameNode, body);
ExternalModuleDeclaration decl = new ExternalModuleDeclaration(loc, (Literal) nameNode, body);
attachSymbolInformation(decl, node);
System.out.println("ExternalModuleDeclaration symbol = " + decl.getSymbol());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug print? Remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦

/**
* Gets the `types` or `typings` field of this package.
*/
string getTypings() { result = this.getPropStringValue(["types", "typings"]) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also the "types" field under "exports": https://devblogs.microsoft.com/typescript/announcing-typescript-4-7-rc/#package-json-exports-imports-and-self-referencing

(Might not be that relevant, so you can probably skip it).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new exports/imports fields are too complex to handle in this PR

@asgerf asgerf merged commit 2936e1a into github:main Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JS no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants