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 crash when export declaration has no local property #93

Merged
merged 2 commits into from Oct 29, 2020

Conversation

tremby
Copy link
Contributor

@tremby tremby commented Oct 29, 2020

Closes #92

@lencioni
Copy link
Member

Thanks for the PR! Can you please add a regression test?

@tremby
Copy link
Contributor Author

tremby commented Oct 29, 2020 via email

@lencioni
Copy link
Member

In your issue, your AST showed it was encountering an ExportNamespaceSpecifier. This happens when you have code like this:

export * as foo from 'somewhere';

The ExportNamespaceSpecifier is the "as foo" part.

This code appears to be looking for a pattern like this:

export { default as bar } from 'somewhere';

Where the default export from 'somewhere' is being re-exported as the named specifier bar.

You can see this in this AST explorer here: https://astexplorer.net/#/gist/94c2a82f4d113bd01e39f4078a738f7d/adffef78643a0efd2bf9c1c59ef05456ca3838ab

I believe the fix you have here will prevent the plugin from crashing on the ExportNamedSpecifier in my first example, which is an improvement. A test should be added for this case, that would have failed without your fix.

As for whether we want to do something to handle that in a better way, I'm not really sure! I think probably. It would be great if you could dig into this a little more.

@tremby
Copy link
Contributor Author

tremby commented Oct 29, 2020

I've added a test which fails before the patch and passes after.

I did start to have a look deeper but I don't even know what the function is meant to do. It's not at all clear to me for example why it's currently only running applyPlugin in this case if .local.name is equal to default.

@lencioni
Copy link
Member

Thanks!

@lencioni lencioni merged commit 7706035 into airbnb:master Oct 29, 2020
@lencioni
Copy link
Member

v1.1.2 has been published with this fix!

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.

App crashing with "Cannot read property 'name' of undefined" after installing plugin
2 participants