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

feat: extend babilon support #22

Merged
merged 1 commit into from
Mar 27, 2017
Merged

Conversation

jonathanargentiero
Copy link
Contributor

add support for jsx syntax, flow annotations, async generators and dynamic imports

add support for jsx syntax, flow annotations, async generators and dynamic imports
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 91.214% when pulling 94c677c on jonathanargentiero:master into 4d71d5e on istanbuljs:master.

@jonathanargentiero
Copy link
Contributor Author

This is the continuation of istanbuljs-archived-repos/istanbul-lib-instrument#42
I would have waited for the OP pull request but in case he forget I would like to have this feature included as soon as possible.

dynamicImport is become essential since Webpack deprecated System.import in favor of import (more details here). Instabul-lib-instrument doesn't work without that flag enabled.

Copy link
Member

@JaKXz JaKXz left a comment

Choose a reason for hiding this comment

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

This makes sense to me - I'm thinking it's possible to write a test for this so that would be great to see. In any case, thanks!

'dynamicImport',
'flow',
'jsx'
]
Copy link
Member

Choose a reason for hiding this comment

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

Is there some way we can just pull this list in, like, is it exported from babylon or something?

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 list is taken from here. You can find all the "experimental" features of babylon under their test folder here. Adding some test won't be a problem, expecially if we take inspiration from their material. However I am not familiar enough with the plugin to write them.

Copy link
Member

Choose a reason for hiding this comment

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

@jonathanargentiero @JaKXz I don't know enough about how plugins are implemented to gauge how safe this is (@hzoo perhaps you could give this a quick 👀 ).

When are the plugins actually loaded, are they shipped as part of babylon? lol, explain this change to me like I'm five.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bcoe This plugins are already part of babylon and flagged as experimental language proposal. These plugins are at least stage-0 proposal and activating them means enabling new parsing rules that are out of standard ES6 spec.

They tend to be newer feature, so yeah, the stability is ensured by the tests on the babylon repo but it won't be reliable as older feature.

Anyway this repository is already using this feature: https://github.com/istanbuljs/istanbuljs/blob/master/packages/istanbul-lib-instrument/src/read-coverage.js#L17

plugins: ["*"] is a soon to be deprecated way to call all babylon plugins. If you think that that part of code is stable then I suppose you can consider stable also this PR.

Copy link

Choose a reason for hiding this comment

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

Sorry I havent had time to work more on the original pr. Either way I was looking at the tests there, the yaml specs and to be fair I don't know enough about how the instrumenter works to put together a test there, the whole expected thing seems cryptic, it generates a bunch of numbers encoded in json - is this documented somewhere exactly what that is? I mean, I have a hunch its lines/columns etc - but yeah not really sure. If someone else could pitch on with some idea of how to test this, that'd be awesome - adding another spec with jsx there etc.

Copy link
Member

Choose a reason for hiding this comment

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

@Axbon @jonathanargentiero if you don't beat me too it, I'm hoping to do a bit of coding this afternoon and will see if I can't get a test around this -- something testing jsx seems like it would be a good exa,mple in the codebase.

Copy link
Contributor

Choose a reason for hiding this comment

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

Babylon plugins aren't the same as Babel plugins.

Babylon "plugins" just let you parse more stuff (they aren't separately modules/packages; they are included in babylon (doesn't have any dependencies) and are not on by default since non of the plugins are in the Javascript spec (until Stage 4)

and yes * is a bad idea because we may update the experimental proposals since they will change over time.

If you want to be able to parse everything then you can add all the plugins in the list. The estree plugin will back babylon parse with https://github.com/estree/estree

@bcoe bcoe merged commit 11c2438 into istanbuljs:master Mar 27, 2017
arnabsen pushed a commit to arnabsen/istanbuljs that referenced this pull request Feb 20, 2024
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.

None yet

6 participants