Medium / phantomjs

NPM wrapper for installing phantomjs
Other
1.43k stars 435 forks source link

bundledDependencies is producing a long path under node_modules #589

Closed siirila closed 8 years ago

siirila commented 8 years ago

The bundledDependencies are producing a long path length that can cause problems on Windows if the path before the node_modules folder is ~70 characters long.

It looks like the change for #585 is resulting in the node_modules folder not being deduped, probably since this appears to be build with npm version 2.15.1, which can cause problems on Windows. Installing the package in a folder name with a single character, such as C:\I, on Windows still results in a path length for the longest path in the node_modules folder of 192 characters. It is easy to exceed the 260 character file path limit as a result, which can cause unexpected behavior.

rmg commented 8 years ago

Good catch. Publishing with bundled deps is definitely something that is ideally done with npm@3. I hope @nicks doesn't mind the extra hurdle this requires.

nicks commented 8 years ago

Ya will do just going to wait for some other PRs to roll in

On Aug 2, 2016 3:41 PM, "Ryan Graham" notifications@github.com wrote:

Good catch. Publishing with bundled deps is definitely something that is ideally done with npm@3. I hope @nicks https://github.com/nicks doesn't mind the extra hurdle this requires.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Medium/phantomjs/issues/589#issuecomment-237019443, or mute the thread https://github.com/notifications/unsubscribe-auth/AARAcds__leba8GcTKE5gpX7oR_U7qz3ks5qb51XgaJpZM4Ja4Vb .

siirila commented 8 years ago

Awesome. Thanks for being so responsive. As an alternative, I looked around a little and it seems like if the nested dependencies were listed in the bundledDependencies list and dedupe was run with npm 2 it should also produce a flattened dependency tree.

I can give that a shot and submit a pull request if you would like, but I wasn't sure if it would produce a different set of dependencies based on the platform.

hem19 commented 8 years ago

even after taking v2.1.10, nested node_modules are getting created and is causing 260 character limit issue in windows

nicks commented 8 years ago

try 2.1.11. TIL that bundledDependencies is a minefield of undocumented assumptions

nicks commented 8 years ago

ya, seems fine in the latest version.