-
Notifications
You must be signed in to change notification settings - Fork 295
⬆️ npm@3.9.3 #562
⬆️ npm@3.9.3 #562
Conversation
Fixes #463 |
@atom/core @damieng @atom/feedback - this is ready for review 👀 , thoughts and feedback welcome! |
Okay, this uses a new installation strategy that we came up with with @kevinsawicki. Previously, we installed packages by creating a blank folder with a
With the new npm v3 package flattening mechanism, we now also get a large number of the package's dependencies at the same level, making it difficult to move the package and all its dependencies into
It's not enough to just move all the other folders into the package's Instead, we now unpack the downloaded Additionally, since npm no longer installs the base This feels pretty heavy handed, but seems to be working, and npm@3 brings some huge wins, so we'd love to get some 💭 around this, and on the caching decision. Thanks! |
Thanks for taking this on, @BinaryMuse and @kuychaco! 👏 ❤️ 💯
Can we delete it entirely? We can put it back at some point in the future, in case things change.
👍 on the caching decision: so long as package dependencies are cached, I think we're good. I am a bit more concerned about removing lifecycle hook scripts, although I guess there's always a defined set of them and we should be fine as long as we remove them all. Thinking more about this, I wonder if we could use a variation of the approach we're taking right now on master: the nice thing about it is that it lets So, what about doing exactly the same thing we were doing before, but pass the
What this means is that we can just copy the folder, but still have the deduping happen on a per-package basis (which, if I understand correctly, seems to be what we would be doing with this pull-request approach too). Thoughts? 💭 |
So despite the work put into the diagrams, they're simply not necessary because @as-cii seems to have tracked down the flag we were looking for through all the RTFM-ing we did yesterday. Not sure how we missed it. :) Looks like the flag turns off package flattening for the first-level of @kuychaco Take a look at the new diff; if you're happy we can |
ae0c6db
to
bbc325b
Compare
Fixes #463
Fixes #322
Fixes #436