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

Replace JSONStream with jsonstream #1247

Merged
merged 1 commit into from May 7, 2015
Merged

Conversation

chadhietala
Copy link
Contributor

NPM now has opinions about names of modules when publishing. We have an internal NPM registry and cannot sync browserify to is due to this dependency. No fear, @dominictarr has published a lowercase version.

NPM now has opinions about names of modules when publishing. We have an internal NPM registry and cannot sync browserify to is due to this dependency. No fear, @dominictarr has published a lowercase version.
@zertosh
Copy link
Member

zertosh commented May 6, 2015

@chadhietala can you make PRs in these deps too? mention me and I'll do them in one big batch.

browser-pack
deps-sort
insert-module-globals
module-deps

EDIT: sorry hit send too soon.

@tleunen
Copy link

tleunen commented May 7, 2015

Does it mean people who install JSONStream are using this old version? https://www.npmjs.com/package/JSONStream

Didn't know it was case sensitive...

@zertosh
Copy link
Member

zertosh commented May 7, 2015

@tleunen
Copy link

tleunen commented May 7, 2015

Oh I see, thanks!

chadhietala added a commit to chadhietala/browser-pack that referenced this pull request May 7, 2015
chadhietala added a commit to chadhietala/deps-sort that referenced this pull request May 7, 2015
chadhietala added a commit to chadhietala/insert-module-globals that referenced this pull request May 7, 2015
chadhietala added a commit to chadhietala/module-deps that referenced this pull request May 7, 2015
chadhietala added a commit to chadhietala/deps-sort that referenced this pull request May 7, 2015
@chadhietala
Copy link
Contributor Author

@zertosh Should be all set.

@renestalder
Copy link

Seems I'm not able to install browserify at the moment with latest NPM version and all stuff up to date. I get following error on npm install browserify:

npm ERR! Darwin 14.3.0
npm ERR! argv "node" "/usr/local/bin/npm" "install" "browserify"
npm ERR! node v0.10.32
npm ERR! npm  v2.9.0
npm ERR! code ETARGET

npm ERR! notarget No compatible version found: JSONStream@'>=1.0.3 <2.0.0'
npm ERR! notarget Valid install targets:
npm ERR! notarget ["0.0.0","0.1.0","0.1.1","0.1.2","0.1.3","0.2.0","0.2.1","0.2.2","0.2.3","0.3.0","0.3.1","0.3.2","0.3.3","0.4.0","0.4.1","0.4.2","0.4.3","0.4.4","0.5.0","0.6.0","0.6.1","0.6.2","0.6.3","0.6.4","0.7.0","0.7.1","0.8.0","0.7.2","0.7.3","0.7.4","0.8.1","0.8.2","0.8.3","0.8.4","0.9.0","0.10.0"]
npm ERR! notarget
npm ERR! notarget This is most likely not a problem with npm itself.
npm ERR! notarget In most cases you or one of your dependencies are requesting
npm ERR! notarget a package version that doesn't exist.
npm ERR! notarget
npm ERR! notarget It was specified as a dependency of 'deps-sort'
npm ERR! notarget

@zertosh
Copy link
Member

zertosh commented May 7, 2015

Yeah... i'm still updating the deps

@zertosh
Copy link
Member

zertosh commented May 7, 2015

I'm having trouble with the tests in module-deps

@zertosh
Copy link
Member

zertosh commented May 7, 2015

Blah... the tests are going to fail for a bit because they have devDeps between each other

@zertosh
Copy link
Member

zertosh commented May 7, 2015

The insert-module-globals tests -_-

@terinjokes
Copy link
Contributor

@zertosh does it break operation, or just a temporary annoyance?

@zertosh
Copy link
Member

zertosh commented May 7, 2015

@terinjokes I'm not sure why the insert-module-globals tests stall out

@zertosh
Copy link
Member

zertosh commented May 7, 2015

oh i see

@kriswallsmith
Copy link

👍

zertosh added a commit that referenced this pull request May 7, 2015
Replace JSONStream with jsonstream
@zertosh zertosh merged commit b2284e1 into browserify:master May 7, 2015
zertosh added a commit that referenced this pull request May 7, 2015
@zertosh
Copy link
Member

zertosh commented May 7, 2015

Published as browserify@10.1.2

@zertosh
Copy link
Member

zertosh commented May 7, 2015

@terinjokes Oh fuck... so you have to pull the latest of everything or npm craps out... basically this makes any previous version of browserify uninstallable... rollback, and then bump up the major... yes?

@jmm
Copy link
Collaborator

jmm commented May 7, 2015

Doesn't that bug far precede even npm@2.8.4?

@robhuzzey
Copy link

@zertosh would it be crazy to suggest using the git path in the package.json? (git+ssh://git@github.com:dominictarr/JSONStream.git#1.0.1)

@zertosh
Copy link
Member

zertosh commented May 7, 2015

I think this would work for mac users on older versions of browserify:

  1. Revert the updates in browser-pack, deps-sort, insert-module-globals, and module-deps and publish them as semver patch.
  2. Revert the update in browserify and publish it as semver patch.
  3. Re-apply the updates in browser-pack, deps-sort, insert-module-globals, and module-deps and publish them as semver major
  4. Re-apply the update in browserify and publish it as semvar major

But only if @substack agrees. I don't know what our policy is on OS specific workarounds for external bugs.

EDIT: When I say "revert" I mean like proper git revert so the history reflects what really happened.

@luv2code
Copy link

luv2code commented May 7, 2015

it's not just mac. Also getting the problem on windows.

npm version 2.9.0 + iojs 2.0

@luv2code
Copy link

luv2code commented May 7, 2015

Sorry. 10.1.2 installed without complaint on windows.

@zertosh
Copy link
Member

zertosh commented May 7, 2015

@luv2code Yeah 10.1.2 is fine regardless of OS and npm version... it's only older versions of browserify with (any version?) of npm

@zertosh
Copy link
Member

zertosh commented May 7, 2015

Ok so anything between 5.0.0 and 10.1.1 won't install.... but 4.1.11 (and below) and 10.1.2 (going forward) do

@jmm
Copy link
Collaborator

jmm commented May 7, 2015

@zertosh I don't fully understand the issue, and I'm sure you don't have time to spell it out right now, but let me know if there's something I can do to help and I'll do what I can.

@zertosh
Copy link
Member

zertosh commented May 7, 2015

@jmm explanation:

JSONStream has been stuck on 0.10.1 because npm/npm#7195 won't allow @dominictarr to update it to 1.0.3. As a workaround, he published it as jsonstream (lowercase) (see dominictarr/JSONStream#59) - which is an entirely new package.

browserify, browser-pack, deps-sort, insert-module-globals, and module-deps, each depend on "JSONStream". I updated each of them so they used jsonstream@^1.0.3 instead of JSONStream@~0.10.0. The updates were all semver patch.

If someone tries to install a browserify that uses "JSONStream@~0.10.0", but has a dep that uses jsonstream@^1.0.3 (because the sub deps fall within the allowed semver range), npm will crap out. It seems that once it sees "JSONStream@~0.10.0" it tries to install "jsonstream@^1.0.3" as "JSONStream@^1.0.3" - which doesn't exist. It must be something with case nature of the fs because this doesn't happen on linux.

@jmm
Copy link
Collaborator

jmm commented May 7, 2015

@zertosh Thanks for the info.

It seems that once it sees "JSONStream@~0.10.0" it tries to install "jsonstream@^1.0.3" as "JSONStream@^1.0.3" - which doesn't exist. It must be something with case nature of the fs because this doesn't happen on linux.

Ah, ok, this was the part I couldn't figure out. I thought that npm/npm#7195 only affected the publishing side (and predates v2.8.x). Is there an npm issue for this problem?

@emperorcezar
Copy link

I've fixed the issue on my end by setting "jsonstream": "1.0.3", as a dependency in my package.json file.

@zertosh
Copy link
Member

zertosh commented May 7, 2015

I thought that npm/npm#7195 only affected the publishing side (and predates v2.8.x). Is there an npm issue for this problem?

It only affects publishing, but as an unintended consequence, the hack/workaround (rename your package) turned out to have more bugs.

Is there an npm issue for this problem?

Could be this one? npm/npm#7933

@zertosh
Copy link
Member

zertosh commented May 7, 2015

I've fixed the issue on my end by setting "jsonstream": "1.0.3", as a dependency in my package.json file.

@emperorcezar WOW... so there must be a race condition in npm (tried v2.9.0), where this "trick" works sometimes. My success rate was ~50%. Just have to try a few times 😡

@zertosh
Copy link
Member

zertosh commented May 7, 2015

@dominictarr Has saved the day! dominictarr/JSONStream#68 (comment). I get a few warnings but browserify still works:

npm ERR! missing: jsonstream@^1.0.3, required by browser-pack@4.0.3
npm ERR! extraneous: JSONStream@1.0.3 /Users/andres/Downloads/node_modules/browserify/node_modules/browser-pack/node_modules/JSONStream
npm ERR! missing: jsonstream@^1.0.3, required by deps-sort@1.3.7
npm ERR! extraneous: JSONStream@1.0.3 /Users/andres/Downloads/node_modules/browserify/node_modules/deps-sort/node_modules/JSONStream
npm ERR! missing: jsonstream@^1.0.3, required by insert-module-globals@6.4.1
npm ERR! extraneous: JSONStream@1.0.3 /Users/andres/Downloads/node_modules/browserify/node_modules/insert-module-globals/node_modules/JSONStream
npm ERR! missing: jsonstream@^1.0.3, required by module-deps@3.7.11
npm ERR! extraneous: JSONStream@1.0.3 /Users/andres/Downloads/node_modules/browserify/node_modules/module-deps/node_modules/JSONStream

@jmm
Copy link
Collaborator

jmm commented May 7, 2015

@zertosh thanks, npm/npm#7933 does sound similar. I guess it's irrelevant though.

@zertosh
Copy link
Member

zertosh commented May 7, 2015

@jmm yeah... sigh... i don't know... i'm gonna go back to writing css or something

@terinjokes
Copy link
Contributor

@zertosh :rage1:

@zertosh
Copy link
Member

zertosh commented May 7, 2015

Ok it's all been re-updated and published with JSONStream as browserify@10.1.3, browser-pack@4.0.4, deps-sort@1.3.8, insert-module-globals@6.4.2, and module-deps@3.7.12 (#1251).

@renestalder
Copy link

browserify@10.1.3 installed on OSX and seems to work like a charm.

Where is the “buy these guys a beer” button?

@robhuzzey
Copy link

browserify@10.1.3 on OSX working for me too... awesome work! Thanks :)

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

Successfully merging this pull request may close these issues.

None yet

10 participants