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

fix: handling export declarations #80

Merged
merged 2 commits into from
May 9, 2024
Merged

fix: handling export declarations #80

merged 2 commits into from
May 9, 2024

Conversation

noqcks
Copy link
Contributor

@noqcks noqcks commented May 9, 2024

handling export declarations as top level items. We weren't handling this before, so if an error occurred on one of these lines, we would be able to fix the error.

please take a look at the test snapshot

ts ast example

Signed-off-by: Benji Visser <[email protected]>
@noqcks noqcks requested a review from zlalvani May 9, 2024 01:12
// handling the case of 'export * from "x"' where the name is __export
// for all exports of this type, but we need to make sure ids are unique
// in the graph
if (n.getKind() === SyntaxKind.ExportDeclaration && name === "__export") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

might eventually split this out into its own branching logic per node type if it gets more complicated, starting to smell

even this line is smelly

const nodeName = "getName" in n ? n.getName() : n.getSymbol()?.getName();

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should fix this when we revamp the ID approach (map nodes by similarity)

@noqcks noqcks merged commit 4d994ba into main May 9, 2024
8 checks passed
@noqcks noqcks deleted the noqcks/XEO-444 branch May 9, 2024 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants