-
-
Notifications
You must be signed in to change notification settings - Fork 266
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
docs: updated contributing instructions #3376
base: main
Are you sure you want to change the base?
docs: updated contributing instructions #3376
Conversation
|
✅ Deploy Preview for module-federation-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -6,7 +6,7 @@ Thank you for your interest in contributing to Module Federation! Before startin | |||
|
|||
1. [Fork](https://help.github.com/articles/fork-a-repo/) the Module Federation repository into your own GitHub account. | |||
2. [Clone](https://help.github.com/articles/cloning-a-repository/) the repository to your local machine. | |||
3. Checkout a new branch from `main` or `canary`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no canary
branch
pnpm-lock.yaml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too many changes here, probably pnpm 9 lockfile is not compatible with v8
@2heal1 You can take care of this |
package.json
Outdated
@@ -2,8 +2,8 @@ | |||
"name": "module-federation", | |||
"version": "0.0.0", | |||
"engines": { | |||
"node": "^18", | |||
"pnpm": "^8.11.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we use corepack, should us exact pnpm version otherwise this can cause issues
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, how about node
, is 18
mandatory?
I have multiple projects and some expect v20.
Should we containerize module-federation or is there a tool, similar to corepack
, that can auto-swap node binaries based on engines
section in package.json? Or is the idea to run nvm use 18
on every newly opened terminal (considering that the default
version is pointed to the lts v20)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are also troubles when using corepack (no wonder it is in Experimental stage)
> pnpm -v
9.13.0
> corepack cache clean
> corepack disable
> corepack enable
> corepack install
Adding [email protected] to the cache...
> pnpm -v
9.13.0
> corepack prepare [email protected] --activate
Preparing [email protected] for immediate activation...
> pnpm -v
9.13.0
That is on Ubuntu WSL
So I went with rm
-ing all instances that were detected with whereis pnpm
> rm /home/stevenp/.nvm/versions/node/v18.20.5/bin/pnpm
> rm -rf /home/stevenp/.local/share/pnpm
> rm -rf ~/.pnpm-store
> corepack install
That worked, but if I would need a different pnpm version in another project, I might face with issues again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevertheless, please check the updated PR
60a072d
to
5f057bb
Compare
@@ -30,7 +30,6 @@ To install Node.js, use [nvm](https://github.com/nvm-sh/nvm) or [fnm](https://gi | |||
```bash | |||
# Install Node.js 18 LTS | |||
nvm install 18 --lts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved suggestion to use node as default version in a separate section
88b820e
to
a5394f8
Compare
a5394f8
to
db8ef71
Compare
db8ef71
to
ab65f90
Compare
Description
Updated development requirements and contributing guidelines:
corepack
corepack
canary
branchAddresses #3375
Although it is possible to use both pnpm 8 and pnpm 9 with
corepack
, for node itself one should usenvm use
in every new instance of the terminal (or swap thedefault
)Related Issue
N/A - Maintenance update
Types of changes
Checklist