-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
1.10.1 breaks ESLint 2.13.1 and Node < 4 #415
Comments
I'm also running into a problem with 1.10.0 where if the packages were installed with NPM 2.14.22, they work under either NPM 2.14.22 or under NPM 3.9.6. But if they are installed with NPM 3.9.6, I get the following.
This plus the above problem leaves me in a position of not knowing how to get this to work with Meteor v1.3.4.1's combination of NPM 3.9.6 and Node 0.10.45. I can't really tell all of the users to run one NPM for the npm install and another for everything else. |
Bizarre. Nothing in master should fail against Node 0.10. Travis would have complained. I suspect I botched the publish somehow when I was switching between master and v2 branches. I will take a look in the morning. 1.10.x should support Node 0.10. |
Actually I was too terrified to let it go until morning. Something horrible definitely happened with my publish of 1.10.1. Republished as 1.10.2. |
Thank you! That will stop problems with those of our users who have not yet moved up to our latest version. I verified that the Meteor 1.3 products that use Node v0.10.43 and npm v2.14.22 are happy with eslint 2.13.1 and eslint-plugin-import 1.10.2. The narrower issue in my second comment does, however, persist. This seems to be a factor of installing with NPM v3.9.6 and running with NPM v3.9.6 and Node v0.10.43. If I run with Node v4.4.7 instead, even without reinstalling, the problem goes away. This effects anyone using the latest released version of Meteor, v1.3.4.1. Looking back, I see that I probably should have given you the output, not the npm-debug.log. Here is the output running with eslint-plugin-import 1.10.2 that indicates an issue at eslint-plugin-import/lib/rules/export.js:93:149
|
👍 thanks for the quick update. All good now. |
@rhettlivingston ah, sorry. got tunnel vision after seeing |
Strange. I'm not sure how that would happen. Sounds like a weird bug with the Travis installs with |
Is there an implication here that you don't see this issue with |
I marched this all the way back to eslint-plugin-import 1.7.0 - always the same problem. Here is the translated code in case it helps. 'Program:exit': function ProgramExit() {
for (var _iterator2 = named, _isArray2 = Array.isArray(_iterator2), _i2 = 0, _iterator2 = _isArray2 ? _iterator2 : _iterator2[Symbol.iterator]();;) {
var _ref2; Also, in case it matters, I appear to have es-map 0.1.4 and es-symbol 3.1.0.
What is the code doing at that point? It looks to me as if it is trying to traverse an es6-map of named exports that it has collected in linting a module. We certainly do have a lot of named exports in our code. But, the routine is titled "ProgramExit", so is this actually a check after linting all modules? |
Just to be sure... I just duplicated this completely outside of the meteor environment with a very stripped down package.json and basically empty .js file as follows:
|
OK. Progress. You may get off the hook. If I don't tell ESLint about the meteor plugin, eslint-plugin-meteor, the problem goes away. The following package.json works even though I still included eslint-plugin-meteor in the devDependencies. I just pulled out the lines telling ESLint to use it. I will open a parallel issue on eslint-plugin-meteor to try to bring @dferber90 into the conversation. They may be doing something that breaks your module.
|
I wonder if we're both globally implementing I felt dirty using |
Looks like I can imagine them politely coexisting, though. Could be a red herring... |
(this is part of why I'd like to get away from all but syntax transpilation for this plugin...) |
I haven't read through the whole issue. ESLint-plugin-meteor uses ESLint-plugin-import internally.
|
Very possible. They are using babel. On Tue, Jul 5, 2016 at 2:42 PM, Ben Mosher notifications@github.com wrote:
|
Oddly enough. It works with npm 2, not npm 3. Also, if I npm install with 2 and run with 3, it will work. Only npm install with 3 and run with 3 breaks. Look closer at the most recent log comments above...
Note no node-modules installed,,, then ... rhett@dreamshot: ... npm ERR! Linux 4.4.0-28-generic
|
Disregard my last comment. eslint-plugin-import is a I'm puzzled about the issue 🙄 |
Just a reminder that Node is a component of the issue too. Might be a clue to someone.
|
Oof, I don't know why I thought this was a good idea: https://github.com/benmosher/eslint-plugin-import/blob/master/package.json#L73 It's probably the issue. I bet if you |
No luck. I pushed my super-simple demo to https://github.com/rhettlivingston/debug-eslint-plugin-import so that others may easily work with this issue. |
Bummer. had high hopes. I've got a branch where I've started removing |
I'm curious,,, if you removed just that one,,, did the problem shift to I saw the branch. May give it a shot shortly. On Wed, Jul 6, 2016 at 12:15 PM, Ben Mosher notifications@github.com
|
I expect that it would, but it's tough to say. The |
The bad news is that it doesn't just fix it. The good news is that the problem moved to
So, this approach shows promise for fixing the issue if carried through. |
I'll accept PR(s) for that branch if you or others are willing to refactor the handful of places it's used. AFAICT there are good tests wrapped around all the relevant places. Let me know if you're up for it. I can do it, but not sure when. |
Giving it a shot. On Wed, Jul 6, 2016 at 2:00 PM, Ben Mosher notifications@github.com wrote:
|
This dependancy version has been unreleased because of a bug introduced by the maintener import-js/eslint-plugin-import#415
Thank you again for accepting the PR! Any forecast on the patch date? I'm waiting to release updates to the Meteor sample app that require this change. I thought about releasing them with a github reference in the package.json, but that kind of reference doesn't seem to trigger the build on your module during npm install. |
Yeah, I'm trying to get away from transpiling for a number of reasons. The botched publish and that inability to install from Github are two big ones. I nearly published on Friday, but in my quick QA I found an issue with a I'll try to get it out this week, just want to make sure I don't botch another publish! 😅 |
Wow, my apologies if that was my 'foreach'! I actually had the thought to
double-check that because I caught myself doing it once (and then forgot to
do so). But the coverage was seeming pretty good. It caught me in other
errors many times over.
Anyway, thank you again!
|
Confirmed all good on 1.10.3! Thanks!! |
I work with Meteor which is just starting to adopt Node 4 (in Beta). We have many sample apps running Node 0.10.x.
I just downloaded the main Meteor todos app, ran "npm install" and got eslint-plugin-import 1.10.1 installed with ESLint 2.13.1. This produced the following error...
I'm no NPM expert, but do you perhaps need a node 4 requirement in your package.json now?
Reverting to 1.10.0 fixed my problem. Also, running with Node 4 fixed it, but that isn't something our user base at large is ready for yet.
The text was updated successfully, but these errors were encountered: