search

guybedford / es-module-lexer

Low-overhead lexer dedicated to ES module parsing for fast analysis
MIT License
917 stars 48 forks source link

Changes exports to ExportSpecifier. #119

Closed tommie closed 2 years ago

tommie commented 2 years ago

Closes https://github.com/guybedford/es-module-lexer/issues/116.

Adds information about the local name for "export { a as b }" and whether it's an ambient export or not.

Breaking change for the parse() API since exports is no longer an array of strings.

guybedford commented 2 years ago

I'm wondering if I've fully thought through the "ambient" concept. I guess the point is to try and work out when a local binding is live in that it could change. Perhaps the local binding existing at all is the only thing that affects that though I suppose? In which case any if (ambient) check could just be if (localBinding)? Apologies for introducing confusion there if I didn't fully think it through!

As for the identifier thing with default, the point here is that default is not a valid identifier therefore shouldn't be treated as "live" either. BUT again I think I've mis-assigned the issue here in that this is fully captured by localName since it will always be a valid local identifier by definition. So for both ambient and the identifier checks, I believe I'm completely wrong and localName being defined is all we need? Please double check and confirm I've got this right now?

I had a look at the CI issues. The Windows failure was a CI regression, which I've just removed for now. But the other failure seems specific to the PR. While I could get the tests running locally it may be some other kind of CI issue. Will look into it further soon. In the mean time, let me know if the above localName alignment makes sense to you. Thanks again for the work on this!

guybedford commented 2 years ago

Released in 1.0.0.

tommie commented 2 years ago

Thanks for getting this merged so quickly!

I just realized that export default function g() {} should probably set ln = "g". At least nodejs seems to make g available locally. I don't think we have any use for this in es-module-shims, and the complication of tryParseExportStatement() is probably not worth it.

Edit: and export default class A {}.

guybedford commented 2 years ago

Yes, you're right on these. I think we should do that, since they are actually assignable as well would you believe it. But that can be a follow-up fix anytime.