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

v2.17.0 introduces a regression #149

Closed
rpadovani opened this issue Dec 18, 2017 · 41 comments
Closed

v2.17.0 introduces a regression #149

rpadovani opened this issue Dec 18, 2017 · 41 comments

Comments

@rpadovani
Copy link

rpadovani commented Dec 18, 2017

I do not use this package directly, but xo does.

After you released 2.17.0 xo doesn't run anymore with this error:

If you break retro-compatibility, you are supposed to to a major version bump.

/builds/***/node_modules/is-my-json-valid/index.js:366
      visit(name+'['+i+']', node.items, reporter, filter, schemaPath.concat('items'))
                                                                     ^

TypeError: Cannot read property 'concat' of undefined
    at visit (/builds/***/node_modules/is-my-json-valid/index.js:366:70)
    at /build/***/node_modules/is-my-json-valid/index.js:416:9
    at Array.forEach (<anonymous>)
    at visit (/builds/***/node_modules/is-my-json-valid/index.js:409:18)
    at /builds/***/node_modules/is-my-json-valid/index.js:543:9
    at Array.forEach (<anonymous>)
    at visit (/builds/***/node_modules/is-my-json-valid/index.js:540:31)
    at compile (/builds/***/node_modules/is-my-json-valid/index.js:565:3)
    at visit (/builds/***/node_modules/is-my-json-valid/index.js:340:16)
    at /builds/***/node_modules/is-my-json-valid/index.js:416:9
@ambewas
Copy link

ambewas commented Dec 18, 2017

same problem here, and can't fix a version in our package.json due to it being a deeper dependency as well.

@hustcc
Copy link

hustcc commented Dec 18, 2017

/Users/xxx/Documents/git/d-vision/.eslintrc:
	Configuration for rule "no-use-before-define" is invalid:
Cannot read property 'concat' of undefined

Similar error.

如果不兼容,建议升级大版本。

@shuky19
Copy link

shuky19 commented Dec 18, 2017

Same here, using eslint...

@somprabhsharma
Copy link

somprabhsharma commented Dec 18, 2017

Similar error @mafintosh

@Wuvnen
Copy link

Wuvnen commented Dec 18, 2017

same problem here.

@vaail
Copy link

vaail commented Dec 18, 2017

same problem here

1 similar comment
@xiaoyang4011
Copy link

same problem here

@GottfridL
Copy link

GottfridL commented Dec 18, 2017

same problem here using ember-cli-eslint and ember-cli-sass which take latest minor version upgrades.

@mateussmohamed
Copy link

same problem here!

@sebheitzmann
Copy link

+1

@Clement-TS
Copy link

Same here with standardjs

@jasollien
Copy link

Same here

@SonoAlien
Copy link

+1

@Ricardowouda
Copy link

Same problem here.

@Andrei997
Copy link

Same here with Nuxt.

@blgm
Copy link

blgm commented Dec 18, 2017

I'm broken too. As a workaround, you can install the previous version of this module:

npm install [email protected]

This gets StandardJS (and presumably ESLint) working, because they will just pick up the installed @2.16 if it's there.

@deBhal @LinusU, I guess you guys know the code better than most of us, so are probably in the best position to fix it. As a community, is there anything we can do to be helpful?

vaail pushed a commit to vaail/is-my-json-valid that referenced this issue Dec 18, 2017
@vaail
Copy link

vaail commented Dec 18, 2017

@mafintosh @LinusU @deBhal #150

@sant0shg
Copy link

Same issue here

@l86110
Copy link

l86110 commented Dec 18, 2017

Same here

1 similar comment
@ameya-kulkarni
Copy link

Same here

@spotscale
Copy link

Same issue here.

@kalanda
Copy link

kalanda commented Dec 18, 2017

Same here

@vzorge
Copy link

vzorge commented Dec 18, 2017

Same here.

@joachimhb
Copy link

Same here

@shahsank3t
Copy link

Same here.

@ncordin
Copy link

ncordin commented Dec 18, 2017

Same here

2 similar comments
@louischan-oursky
Copy link

Same here

@satishwaghela
Copy link

Same here

@sameer79
Copy link

Same here

@EreMaijala
Copy link

I believe it's already obvious this is a major issue, so I'd like to suggest everyone to just subscribe on this issue instead of adding more "Same here" comments.

@voxpelli
Copy link

Fixed by #151 and released as 2.17.1 – thanks @LinusU for quick fix! 👍

@LinusU
Copy link
Collaborator

LinusU commented Dec 18, 2017

Fix published as 2.17.1, so sorry for any inconvenience caused!

@LinusU LinusU closed this as completed Dec 18, 2017
EreMaijala added a commit to NatLibFi/NDL-VuFind2 that referenced this issue Dec 18, 2017
@LinusU
Copy link
Collaborator

LinusU commented Dec 18, 2017

Super sorry for this 🙈

Will try to add more test coverage and possibly move to TypeScript to prevent this from happening again 👌

@akryvda
Copy link

akryvda commented Dec 18, 2017

Should we expect a hotfix in near future? :)

@LinusU
Copy link
Collaborator

LinusU commented Dec 18, 2017

@akryvda, it's already published: 2.17.1

@akryvda
Copy link

akryvda commented Dec 18, 2017

Thanks, for quick update! Will rerun our pipelines

@peterclifflm
Copy link

My build was failing but now is not. Fast work - thank you!

@LinusU
Copy link
Collaborator

LinusU commented Dec 18, 2017

2.17.0 have been deprecated on npm with the following message:

A bug in this version causes an exception to be thrown when using anyOf or oneOf, please upgrade to 2.17.1 instead

@rpadovani
Copy link
Author

rpadovani commented Dec 18, 2017

Thanks for your fast fix on this, keep up the good work :-)

@LinusU
Copy link
Collaborator

LinusU commented Dec 18, 2017

Would like to take this opportunity to plug lock files 🔒📝

By using npm 5 or newer, or yarn, a package-lock.json (or yarn.lock) file will automatically be created when adding or removing dependencies. This file will contain the exact version of every dependency (even the dependency of your dependencies, and so forth). Whenever npm install (or yarn) is run, it will look for this file and install the exact dependencies as the last time, thus ensuring that your builds are reproducible.

The file should be commited together with the rest of the code, so that everyone that works on the project have the same versions of every dependency, and so that the CI and deploy also uses the same versions.

More reading: "Yarn determinism" blog post, package-lock.json on npm docs, yarn.lock on yarn docs

Again, sorry for any inconvenience, hopefully this wont happen again, but I would advice everyone to start using lock files to minimize the impact if (when 😄) it happens the next time. If anyone is in London I'd be happy to buy you a beer and discuss lock files and JavaScript 😄

Cheers 🍻

@deBhal
Copy link
Contributor

deBhal commented Dec 19, 2017

I'd also like to add my apologies to everyone inconvenienced, and my thanks to @LinusU and @shalevshalit @juliangruber @shalevshalit who all provided fixes so quickly while I was asleep.

Will try to add more test coverage

We did have tests here that looked pretty solid to me, so in addition to a PR here to cover this specific case (#158) I've gone upstream to beef up the json-schema.org test-suite more generally as well (json-schema-org/JSON-Schema-Test-Suite#211)

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

No branches or pull requests