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

Add support for ESLint v2 #107

Merged
merged 1 commit into from
Apr 6, 2016
Merged

Add support for ESLint v2 #107

merged 1 commit into from
Apr 6, 2016

Conversation

Daniel15
Copy link
Contributor

I copied how Babel 6 is handled - Created "modules" under packages which export the correct version of ESLint. Moved shared code into a eslintUtils file.

Pretty much all the transform logic is identical across ESLint 1 and 2. I kept the transformers themselves (eslint1/index.js and eslint2/index.js) separate rather than totally sharing them (eg. a createESlintTransformer factory method) so that it's easier for them to deviate if needed.

@Daniel15
Copy link
Contributor Author

Hopefully the npm package stuff is okay, I kept getting strange permission errors like this when running npm install:

npm ERR! Windows_NT 10.0.10586
npm ERR! argv "C:\\Program Files\\nodejs\\node.exe" "C:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npm-cli.js" "install"
npm ERR! node v5.7.1
npm ERR! npm  v3.6.0
npm ERR! path C:\src\astexplorer\node_modules\eslint2
npm ERR! code EPERM
npm ERR! errno -4048
npm ERR! syscall rename

npm ERR! Error: EPERM: operation not permitted, rename 'C:\src\astexplorer\node_modules\eslint2' -> 'C:\src\astexplorer\node_modules\.eslint2.DELETE'
npm ERR!     at moveAway (C:\Program Files\nodejs\node_modules\npm\lib\install\action\finalize.js:38:5)
npm ERR!     at destStatted (C:\Program Files\nodejs\node_modules\npm\lib\install\action\finalize.js:27:7)
npm ERR!     at FSReqWrap.oncomplete (fs.js:82:15)
npm ERR!
npm ERR! Error: EPERM: operation not permitted, rename 'C:\src\astexplorer\node_modules\eslint2' -> 'C:\src\astexplorer\node_modules\.eslint2.DELETE'
npm ERR!     at Error (native)
npm ERR!  { [Error: EPERM: operation not permitted, rename 'C:\src\astexplorer\node_modules\eslint2' -> 'C:\src\astexplorer\node_modules\.eslint2.DELETE'] parent: 'astexplorer' }
npm ERR!
npm ERR! Please try running this command again as root/Administrator.

It resolved itself if I ran the command again. I think it's just npm being glitchy and not an actual issue. (I've seen the same thing on Linux and on Windows so I don't think it's an OS-specific issue)

@@ -0,0 +1,23 @@
import {defineRule, runRule} from '../../utils/eslintUtils';
Copy link
Owner

Choose a reason for hiding this comment

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

The problem with this is that the transitive dependencies, babel-eslint and babel-core are now loaded on every page load, not only when someone uses the eslint transform.

You could simply load the utils dynamically as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, good catch! I thought Webpack would automatically make a new bundle for every transformers/*/index.js but maybe it doesn't work that way. How does the code splitting work? It is simply the require([..], cb) that does it? I've never used that feature of Webpack.

Copy link
Owner

Choose a reason for hiding this comment

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

Yep, the require([..], cb) does the magic :)

@fkling
Copy link
Owner

fkling commented Mar 31, 2016

Thank you so much for doing this!

@Daniel15
Copy link
Contributor Author

Daniel15 commented Apr 1, 2016

Updated the diff to also dynamically require eslintUtils.

@fkling fkling merged commit 3679016 into fkling:master Apr 6, 2016
@fkling fkling added the deploy pending Marks issues/PRs that are merged into master but are not deployed to any webserver label Apr 6, 2016
@fkling fkling added deployed to production Marks issues/PRs that are deployed to https://astexplorer.net and removed deploy pending Marks issues/PRs that are merged into master but are not deployed to any webserver labels Apr 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed to production Marks issues/PRs that are deployed to https://astexplorer.net
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants