Skip to content
This repository has been archived by the owner on Aug 4, 2021. It is now read-only.

Use parse method from transform context #287

Merged
merged 3 commits into from Mar 5, 2018

Conversation

unstubbable
Copy link
Contributor

@unstubbable unstubbable commented Feb 5, 2018

This depends on rollup/rollup#1945. It removes acorn as a dependency (though it's still a dev dependency for the tests) and uses the parse method that is exposed via the transform context instead.

fixes #275 (by providing the acornInjectPlugins option to rollup)
(maybe others?)
closes #237

TODO:

  • change rollup version to 0.56.0 when it has been published

Those two values are set by rollup and there is no reason to
overwrite them here.
@unstubbable
Copy link
Contributor Author

unstubbable commented Mar 2, 2018

I updated rollup to 0.56 and the tests are passing now. @lukastaegert This is now ready for a review.

Copy link
Member

@lukastaegert lukastaegert left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@lukastaegert lukastaegert added this to the 8.4.0 milestone Mar 5, 2018
@lukastaegert lukastaegert merged commit df762c7 into rollup:master Mar 5, 2018
@deavial
Copy link

deavial commented Mar 5, 2018

is there some sort of config change needed for this. I am using rollup 0.56.4 and am working correctly on rollup-plugin-commonjs 8.3.0. Any version above that (8.4.0, 8.4.1, 9.0.0) breaks the build with parse is not a function

@unstubbable unstubbable deleted the transform-context-parse branch March 5, 2018 21:52
@rockerest
Copy link

rockerest commented Mar 6, 2018

I am having the exact same issue as @victoriafrench.

Any version later than 8.3.0 of this plugin throws parse is not a function.

Edit:
Apologies, I was using grunt-rollup. After installing rollup manually, rollup-plugin-commonjs' peer dependency of rollup>=0.56 was satisfied, which gave me the false sense that it would use that version. I removed my reliance on grunt-rollup and this plugin is now working fine at version 9.0.0.

@lukastaegert
Copy link
Member

@victoriafrench 8.4.1 should actually be working as it is identical to 8.3.0 (if I released it correctly). Otherwise I would assume that you are sharing @rockerest's issue i.e. you are unknowingly consuming rollup via a secondary dependency that pulls in an older version. In this case, please open an issue for that dependency.

If you can confirm that 8.4.1 really does not work, please let me know so that I can attempt another fix.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spread operator in object literal causes SyntaxError
4 participants