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(razzle): add Yarn PnP support #1398

Closed
wants to merge 11 commits into from
Closed

Conversation

merceyz
Copy link
Contributor

@merceyz merceyz commented Aug 25, 2020

What's the problem this PR addresses?

razzle doesn't work with Yarn PnP due to missing dependencies and incorrect assumptions about the node_modules layout.

ref #896

How did you fix it?

Notes
This only adds PnP support to razzle, the various plugins haven't been fixed as some of them require upstream changes

@vercel
Copy link

vercel bot commented Aug 25, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/jared/razzle/7cyafnksh
✅ Preview: https://razzle-git-fork-merceyz-pnp.jared.vercel.app

@fivethreeo
Copy link
Collaborator

Great work, but. Can you do this against the next-awesome branch?

@fivethreeo
Copy link
Collaborator

Also, don't format the code. Makes it easier to see the changes. :)

@merceyz
Copy link
Contributor Author

merceyz commented Aug 26, 2020

These changes (except the PnP plugin) are bugfixes that should go out in the current release. Based on #1377 next-awesome is going to use webpack 5 which doesn't need the plugin which just leaves bugfixes.

Also, don't format the code. Makes it easier to see the changes. :)

Reverted, but master should probably be formatted then so contributors can make sure their PR is consistent easily

@benkingcode
Copy link

Isn't the code formatted on commit? Prettier is listed in lint-staged

@merceyz
Copy link
Contributor Author

merceyz commented Aug 26, 2020

The pattern wasn't matching anything, fixed.
You can cherry-pick that commit into master and run the format so I can rebase and re-run the format on my changes

@fivethreeo
Copy link
Collaborator

Next-awesome will support both webpack 4 and 5. I will do a 3.2 off next-awesome before 4.0 without webpack 5. Then add wepback 5 support and some breakIng changes. So I don’t have to keep two brances in sync.

Strange, pre commit hook does not run for me.

@fivethreeo
Copy link
Collaborator

But I can lint master 😀

@merceyz
Copy link
Contributor Author

merceyz commented Aug 26, 2020

Strange, pre commit hook does not run for me.

It runs for me but it doesn't match any files, with 0eea6ab it matches correctly

@fivethreeo
Copy link
Collaborator

Hm, github desktop does not run pre-commit. But I wont add pnp to master now. It must go in next-awesome.

@fivethreeo
Copy link
Collaborator

#1400

@merceyz
Copy link
Contributor Author

merceyz commented Aug 27, 2020

I was going to do it just haven't had the time yet, but it's fine, will you add me as a co-author before you merge it?
I'll close this

@merceyz merceyz closed this Aug 27, 2020
@merceyz merceyz deleted the pnp branch August 27, 2020 19:32
@fivethreeo
Copy link
Collaborator

fivethreeo commented Aug 27, 2020 via email

@fivethreeo
Copy link
Collaborator

What kind of changes are needed that require upstream changes for plugins?

@merceyz
Copy link
Contributor Author

merceyz commented Aug 28, 2020

When I tested the typescript plugin the upstream dependencies have undeclared dependencies so that needs to be fixed, otherwise it probably works

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

Successfully merging this pull request may close these issues.

3 participants