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

history dependency screwed up since beta5 #218

Closed
texttechne opened this issue Dec 19, 2015 · 15 comments
Closed

history dependency screwed up since beta5 #218

texttechne opened this issue Dec 19, 2015 · 15 comments

Comments

@texttechne
Copy link

As has been mentioned in two other issues (/issues/193, /issues/198), the history dependency is kind of screwed up.

Installing it only shows deep-equal as dependency, history is missing:

redux-router@1.0.0-beta5 node_modules\redux-router
└── deep-equal@1.0.1

Furthermore, inspecting package.json of history dependency shows ... hmm, well, it is not shrinkwrapped, but somehow cached... haven't seen it before (and who is franz?):

{
  "_args": [
    [
      "history@^1.12.0",
      "/Users/franz/Sites/redux-router"
    ]
  ],
  "_from": "history@>=1.12.0 <2.0.0",
  "_id": "history@1.13.1",
  "_inCache": true,
  "_installable": true,
  "_location": "/history",
  "_nodeVersion": "5.0.0",
  "_npmUser": {
    "email": "mjijackson@gmail.com",
    "name": "mjackson"
  },
  "_npmVersion": "3.3.6",
  "_phantomChildren": {},
  "_requested": {
    "name": "history",
    "raw": "history@^1.12.0",
    "rawSpec": "^1.12.0",
    "scope": null,
    "spec": ">=1.12.0 <2.0.0",
    "type": "range"
  },
  "_requiredBy": [
    "/"
  ],
  "_resolved": "https://registry.npmjs.org/history/-/history-1.13.1.tgz",
  "_shasum": "1d0664e667f031d5757ef73e81ef847beb3aedcd",
  "_shrinkwrap": null,
  "_spec": "history@^1.12.0",
  "_where": "/Users/franz/Sites/redux-router",

  ... fine from here on

As mentioned in the other issues, node_modules won't get installed properly.

My guess is, that this has been committed (it actually is possible, if you exclude your node_modules after having committed)...

Beta4 is not affected, but at that point history was a devDep.

@Scarysize
Copy link
Contributor

Hey, this seems to be a problem on my side. Will look into it.
@texttechne what would be your solution for the history dependency. ´devDependency´ seems to cause problems with shrink-wrap, normal dependency seems to have issues with older npm versions...

@texttechne
Copy link
Author

Ok, digged a little deeper. The fault is to be found within npm. Relevant issues:
#209 (and #54 which lead to changing history as dep instead of devDep)

react-router ran into the same issue. They changed their deploy script.
Issue: remix-run/react-router/issues/2195
Fix: remix-run/react-router@3be6a2d

@texttechne
Copy link
Author

@Scarysize In regard to devDep vs. dep: history is a peer dependency in disguise of a devDep (#54 (comment)). So you should revert to declare it as devDep.

Then the question arises how you release redux-router. Manually? At this point a script like that of react-router would come in handy...

@Scarysize
Copy link
Contributor

@texttechne Thanks for making the effort, I will look into react-routers solution. I pushed a branch earlier today with the history module moved back to devDependencies. When I found a solution to the publishing bug, I will merge it.

@Scarysize
Copy link
Contributor

https://github.com/acdlite/redux-router/blob/release-script/release.sh

So i looked at react-routers release script and made simplified version. I think this should work (tested it without the npm publish). @texttechne Check it out :)

@texttechne
Copy link
Author

@Scarysize right now, you have history referenced in devDeps and deps (master as well as branch release-script). It should only be referenced as devDep, since it technically is a peerDependency (see my last comment).

Otherwise I cannot test that with the release script branch: sources are missing, only package.json and node_modules when running npm install...

@Scarysize
Copy link
Contributor

@texttechne So the release script is now obsolete. The npm bug has been fixed, the react-router guys brought this to my attention.
Also we will drop history completely, as will react-router. The only occurrence of a direct import from history is in our sever module, which we will try to work around.

@koba04
Copy link

koba04 commented Jan 4, 2016

@Scarysize
https://registry.npmjs.org/redux-router/-/redux-router-1.0.0-beta6.tgz

redux-router-1.0.0-beta6 is still including node_modules/history in the tarball.
This will be fixed in the next release?

@mjrussell
Copy link
Contributor

@koba04 its actually a bug in NPM itself, not the in the packaging of the app. Upgrading node from v4.2.1 to v4.2.4 solved the problem for me. If you can't upgrade, you might add a small post install script to your package.json that deletes the unnecessary history. Until we have no dependency at all on history (which should be possible after upgrading to react-router 2.x), those are the only solutions.

@Scarysize
Copy link
Contributor

@mjrussell we might want to add this to the README...depending on how far away the next release is.

@koba04
Copy link

koba04 commented Jan 4, 2016

I know this is a npm's bug. However the bug is about relating the npm packaging process, right?
I'm using npm v4.2.4, But an error occurs.

➜  npm i -S redux-router@1.0.0-beta6
npm WARN package.json test@1.0.0 No description
npm WARN package.json test@1.0.0 No repository field.
npm WARN package.json test@1.0.0 No README data
redux-router@1.0.0-beta6 node_modules/redux-router
└── deep-equal@1.0.1
➜  npm shrinkwrap
npm ERR! Darwin 15.2.0
npm ERR! argv "/Users/koba04/.anyenv/envs/ndenv/versions/4.2.4/bin/node" "/Users/koba04/.anyenv/envs/ndenv/versions/4.2.4/bin/npm" "shrinkwrap"
npm ERR! node v4.2.4
npm ERR! npm  v2.14.12

npm ERR! Problems were encountered
npm ERR! Please correct and try again.
npm ERR! missing: install@^0.4.1, required by test@1.0.0
npm ERR! missing: npm@^3.5.2, required by test@1.0.0
npm ERR! extraneous: history@1.17.0 /Users/koba04/Desktop/test/node_modules/redux-router/node_modules/history
npm ERR!
npm ERR! If you need help, you may report this error at:
npm ERR!     <https://github.com/npm/npm/issues>

npm ERR! Please include the following file with any support request:
npm ERR!     /Users/koba04/Desktop/test/npm-debug.log

(A proposal to add custom post install script is fine for me. thanks.)

@Scarysize
Copy link
Contributor

Hm I could try to remove my local history package and republish the module. Wouldn't be much work and I would be interested if this might help.

@Scarysize
Copy link
Contributor

I just republished as 1.0.0-beta7 and tried a shrink-wrap with npm v3.5.2, did not get any errors. @koba04 let me know if this helped.

@koba04
Copy link

koba04 commented Jan 4, 2016

@Scarysize
Thank you for your work!
I confirmed that npm-shrinkwrap works 🎉

@mjrussell
Copy link
Contributor

Nice, sorry didn't realize there was some work to do packaging as well. Glad this worked.

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

4 participants