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

update to use Parser.acorn #102

Merged
merged 1 commit into from
Oct 12, 2019
Merged

Conversation

mysticatea
Copy link
Contributor

@mysticatea mysticatea commented Sep 24, 2019

This PR updates acorn-jsx to use Parser.acorn property that acorn@7.1.0 added. This is to solve the problem acornjs/acorn#870 describes.

The require("acorn-jsx").tokTypes property is as-is, the JSX token types for the acorn of the peer dependency.

This PR adds a static property acornJsx to the enhanced Parser class.

const acorn = require("acorn")
const acornJsx = require("acorn-jsx")
const JsxParser = acorn.Parser.extend(acornJsx())

console.log(JsxParser.acornJsx) //→ an object { tokContexts: {...}, tokTypes: {...} }

That static property exposes the actual JSX token types to other plugins that want to modify JSX tokens. The JsxParser.acornJsx and require("acorn-jsx").tokTypes are the same instance in most situation. But those can be different in the special situation acornjs/acorn#870 describes.

Espree will use the JsxParser.acornJsx to modify tokens.

@mysticatea
Copy link
Contributor Author

@marijnh @RReverser Would you take a look at this PR?

@marijnh
Copy link
Member

marijnh commented Oct 10, 2019

@RReverser Would you be open to moving the npm package to the acorn org, so that I can publish updates? I'm unlikely to seriously maintain it, but I'm willing to merge and publish PRs like this one.

@RReverser
Copy link
Member

I still don't really understand this change on either Acorn side or Acorn-JSX side, but if you're happy with it and it helps, it's fine by me.

@RReverser
Copy link
Member

@marijnh Done (I think).

@RReverser
Copy link
Member

I'm not sure if I'm in that org though?

index.js Show resolved Hide resolved
@marijnh marijnh merged commit cee1344 into acornjs:master Oct 12, 2019
@marijnh
Copy link
Member

marijnh commented Oct 12, 2019

@RReverser It seems our org is called acornjs, not acorn (sorry)... and now this org has it. Can you still reassign that or should I contact the owner of that org and ask if they reassign it?

@ljharb
Copy link

ljharb commented Oct 12, 2019

I would immediately email npm support about that one.

@mysticatea mysticatea deleted the use-parser-acorn branch October 12, 2019 16:51
@marijnh
Copy link
Member

marijnh commented Oct 13, 2019

Well, this finally got released (5.1.0) just now

@mysticatea
Copy link
Contributor Author

Thank you very much!

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.

4 participants