search

alexanderGugel / ied

:package: Like npm, but faster - an alternative package manager for Node
http://alexandergugel.github.io/ied
MIT License
1.99k stars 53 forks source link

Hotfix release needed - Issue with latest rxjs release candidate #178

Closed henryruhs closed 7 years ago

henryruhs commented 7 years ago

Hello,

there is an issue with "rxjs@5.0.0-rc.2" that was released a day ago. This is going to break all IED released since 2.0.0 until 2.3.3. I suggest to release a hotfix as soon as possible. It is not a good idea to use a beta in combination with ^ in the version number - I hope someone learns a lesson from that :-)

Error: Cannot find module 'rxjs/operator/distinctKey'

Broken build: https://travis-ci.org/redaxmedia/redaxscript-download-sync/builds/173742057

Temporary solution is to install the earlier version of rxjs:

- npm install --global ied
- npm install --global rxjs@5.0.0-rc.1
- ied install

Fixed build: https://travis-ci.org/redaxmedia/redaxscript-download-sync/builds/173776343

Solution for package.json:

"rxjs": "rxjs@5.0.0-rc.1"
alexanderGugel commented 7 years ago

Awesome! Thanks.

Fix released as ied@2.3.4.

henryruhs commented 7 years ago

This does not fix anything, because you are using ^ again... lol

alexanderGugel commented 7 years ago

This has been fixed because we're no longer using distinctKey, which has been removed from RxJS.

Unfortunately Node decided to break symlinks as of this PR: https://github.com/nodejs/node/pull/8749#issuecomment-258423665

This means package managers that make heavy use of symlinked dependencies won't work in newer versions of Node anymore. This includes PNPM and Yarn.

alexanderGugel commented 7 years ago

To follow up, this is actually a bigger issue which would need to be addressed by using a different installation strategy that doesn't rely on symlinks.

henryruhs commented 7 years ago

Let me understand this!? Doesn't this remove all the performance benefit of IED?

I suggest you enable distinctKey and switch to rxjs@5.0.0-rc.1 for a temporary working IED?

alexanderGugel commented 7 years ago

No, but we need to move away from symlinks.

I'm not happy about this either and I've been trying to prevent this change in Node from the start.

In fact, the issue addressing this is currently one of the most-commented on issues on the Node issue tracker.

The fundamental problem is that Node used to identify required modules by their real path, rather by the path of their symlink (it currently still does).

Eventually the decision was made to use the path of the symlink for some rather obscure reasons. After this broke all kinds of packages (not just package managers like ied and pnpm), the PR was eventually reverted. To allow people to use the "new" behaviour, a new flag was added.

Edit: I just noticed this is completely unrelated to your issue and that I haven't slept for quite some time. So don't get too confused about my above comment.

zkochan commented 7 years ago

I think it is fine that it is on a flag for now. With enough time we will be able to prove how awesome is the concept of a shared storage for packages!

I did not see any plans about deprecating --preserve-symlinks. Just this comment

However, even if they plan to do it, they will have to do it as a breaking change, so it'll be not sooner than in version 8. Hence we have like a year to make yarn/pnpm popular enough to be taken into account by the Node.js team.

That's why I'd really like to collaborate with you guys to maybe merge pnpm and yarn or have some shared core or specs... to unite forces so to say

alexanderGugel commented 7 years ago

Well, I don't think yarn and ied are going to merge anytime soon. :neckbeard:

That being said, I'm definitely open to the idea of merging pnpm into ied or vice versa. After all, pnpm started out as a fork of ied! So why not go back to the roots? ☀️

zkochan commented 7 years ago

haha, I've meant merging pnpm and ied.