Skip to content
This repository has been archived by the owner on May 14, 2018. It is now read-only.

Rewrite pnpm using reactive programming #8

Closed
wants to merge 53 commits into from
Closed

Conversation

zkochan
Copy link
Member

@zkochan zkochan commented Aug 8, 2017

Breaking changes:

Pure packages are packages that don't rely on external peer deps.
External peer deps are peer deps resolved from upper in the dep
tree.

BREAKING CHANGE:

Peer dependencies are always grouped with the relying packages.
Even when the peer dependencies are top dependencies.
@zkochan
Copy link
Member Author

zkochan commented Aug 18, 2017

All tests pass but pnpm is 2 times slower with these changes. I can investigate it next week but maybe someone experienced with rxjs can notice errors in my "reactive code" as I am really inexperienced with rxjs

depth: options.currentDepth,
installable,

ctx.installs[fetchedPkg.id] = {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be set before running installDependencies()

@zkochan
Copy link
Member Author

zkochan commented Aug 25, 2017

As of now, with these changes, pnpm has the same speed as without them.

@vjpr
Copy link
Contributor

vjpr commented Aug 25, 2017 via email

@zkochan
Copy link
Member Author

zkochan commented Aug 25, 2017

I want to try to make it a little bit faster than the current version (to justify the huge changes). I will probably do this: pnpm/pnpm#841. It will allow saving the shrinkwrap and store index files earlier.

Also, André Staltz suggested using most instead of rxjs. He says sometimes it is a lot faster than rxjs. I might try that... or maybe later as it might be a lot of work.

I'll publish a beta this weekend though.

@zkochan
Copy link
Member Author

zkochan commented Aug 27, 2017

@vjpr I published the changes as next via pnpm@2.0.0-beta.0

@zkochan
Copy link
Member Author

zkochan commented Aug 28, 2017

I reverted the breaking changes. They were not necessary, so this will be a minor version bump in pnpm

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

Successfully merging this pull request may close these issues.

None yet

2 participants