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

rollup.es.js causes eslint to throw parse error, also references undefined symbol. #2704

Closed
coreyfarrell opened this issue Feb 17, 2019 · 2 comments · Fixed by #2705
Closed

Comments

@coreyfarrell
Copy link

  • Rollup Version: 1.2.0
  • Operating System (or Browser): any
  • Node Version: 10.15.0

How Do We Reproduce?

Minimal reproduction at https://github.com/coreyfarrell/rollup-issue. The import/named plugin is used to verify that import statements request valid exports. Last night rollup@1.2.0 caused the linter for vinyl-rollup to fail as the image/named rule needs to parse the rollup ES module - a process which fails.

I'm unclear if it's allowed to create a new global symbol overwriting a symbol created by import but I'm concerned this might have identified an actual error in rollup.es.js. From rollup.config.js:

const expectedAcornImport = /import acorn__default, { tokTypes, Parser( as [\w$]+)? } from 'acorn';/;
const newAcornImport =
	"import * as acorn__default from 'acorn';\nimport { tokTypes, Parser } from 'acorn';";

It looks like the original source contains import acorn__default, { tokTypes, Parser as Parser$1 } from 'acorn'; but the replacement drops as Parser$1. Later in rollup.es.js Parser$1 is referenced although it's undefined due to the import alteration.

Expected Behavior

ESLint should be able to parse dist/rollup.es.js, it should not reference undefined variables.

@lukastaegert
Copy link
Member

Thanks a lot for identifying this! Yes you are right, I messed up here 🙄 Will try to fix it right away!

@coreyfarrell
Copy link
Author

Thank greenkeeper 😄 for re-running CI every time a dependency releases an update. Also amazing response time!

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 a pull request may close this issue.

2 participants