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

feat: public-hoist-pattern #2631

Merged
merged 18 commits into from Jun 15, 2020
Merged

feat: public-hoist-pattern #2631

merged 18 commits into from Jun 15, 2020

Conversation

zkochan
Copy link
Member

@zkochan zkochan commented Jun 13, 2020

close #2628

@zkochan zkochan added this to the v5.2 milestone Jun 13, 2020
@zkochan zkochan force-pushed the public-hoist-pattern branch 2 times, most recently from 2f85911 to 66c5ae6 Compare June 13, 2020 23:42
@zkochan zkochan marked this pull request as ready for review June 15, 2020 18:35
@zkochan zkochan requested a review from a team June 15, 2020 18:35
Copy link
Contributor

@aparajita aparajita left a comment

Choose a reason for hiding this comment

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

One question: can the hoist pattern be specified by a setting in a per-package .npmrc?

@zkochan
Copy link
Member Author

zkochan commented Jun 15, 2020

No, currently hoisting is not possible per package. It can be set once for a monorepo or once for a project with a single package.json

Copy link
Contributor

@aparajita aparajita left a comment

Choose a reason for hiding this comment

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

Once for a monorepo or once for a project with a single package.json is sufficient. So the user would set public-host-pattern=<pattern> in the project .npmrc?

@zkochan
Copy link
Member Author

zkochan commented Jun 15, 2020

Yes. But I think we will be updating the default value of that setting when we will find issues in the ecosystem. This way new users will not get disappointed by pnpm. I think @vjpr might have a list already

Copy link
Contributor

@aparajita aparajita left a comment

Choose a reason for hiding this comment

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

Sounds good, just want to be sure all this is well documented.

@zkochan
Copy link
Member Author

zkochan commented Jun 15, 2020

sorry for all the notifications. Hopefully, now it will pass.

@vjpr
Copy link
Contributor

vjpr commented Jun 15, 2020

we will be updating the default value of that setting when we will find issues in the ecosystem.

This is a really interesting solution.

But usually the fixes for such packages are to add the missing dependency or peerDependency using the pnpmfile to the individual packages. Its just that the pnpmfile is a little too tricky to use.

@zkochan
Copy link
Member Author

zkochan commented Jun 15, 2020

I think there are some buggy pluggable frameworks like angular and karma that might benefit from this feature.

Like angular now only works with shamefully-flatten. Maybe we can come up with a pattern that is not * to fix the angular issues. Or maybe not. I don't know yet.

@vjpr
Copy link
Contributor

vjpr commented Jun 15, 2020

Is there a GH issue open for Angular support?

@aparajita
Copy link
Contributor

See my proposal in #2634 for an elegant way to deal with individual packages that would be too difficult for a pattern to deal with.

@zkochan zkochan merged commit 71a8c8c into master Jun 15, 2020
@zkochan zkochan deleted the public-hoist-pattern branch June 15, 2020 21:50
@aparajita
Copy link
Contributor

Maybe we can come up with a pattern that is not * to fix the angular issues. Or maybe not. I don't know yet.

@zkochan See my proposal in #2634

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

Successfully merging this pull request may close these issues.

feat: public-hoist-pattern
3 participants