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.

Sign up for GitHub

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

Jump to bottom

🔨 feat: Migrate to pnpm #9516

Merged
merged 12 commits into from Dec 5, 2022
Merged

🔨 feat: Migrate to pnpm #9516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
d447a36
🔨 feat: replacing yarn by pnpm
nvh95 Dec 4, 2022
e6ab840
🔨 chore: Update package.json
nvh95 Dec 4, 2022
147254c
🛠️ feat: Update GitHub Actions
nvh95 Dec 4, 2022
b4eaab5
🔨 feat: Install pnpm for compressed-size workflow
nvh95 Dec 4, 2022
3833901
🔨 feat: Use setup-node for compress-size.yml
nvh95 Dec 4, 2022
9f620f4
fix: fix syntax
nvh95 Dec 4, 2022
1030127
🔨 feat: use pnpm link for start command
nvh95 Dec 4, 2022
866f04d
🧪 feat: use pnpm for automation workflow
nvh95 Dec 4, 2022
4d741c5
feat: no need to link react
nvh95 Dec 5, 2022
6034507
chore: add a build:watch script
nvh95 Dec 5, 2022
a68cdfc
feat: do not need to link rhf to `app`
nvh95 Dec 5, 2022
f83aa9a
remove the following script due to overlap
bluebill1049 Dec 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 20 additions & 5 deletions .github/workflows/api-extrator.yml
View file
Open in desktop
Expand Up @@ -5,16 +5,31 @@ on: [pull_request]
jobs:
build:
runs-on: ubuntu-latest
strategy:
matrix:
# Do we want to run with multiple versions of node?
node-version: [16]
Comment on lines +10 to +11
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bluebill1049 It is recommended to specify which node version we use. FYI

Copy link
Member

Choose a reason for hiding this comment

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

sounds good 👍


steps:
- name: Checkout repo
uses: actions/checkout@v2
uses: actions/checkout@v3

- name: Install deps and build (with cache)
uses: bahmutov/npm-install@v1
- name: Setup pnpm
uses: pnpm/action-setup@v2
with:
version: 7

- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node-version }}
cache: 'pnpm'

- name: Install dependencies
run: pnpm install

- name: Build
run: yarn build:esm
run: pnpm build:esm

- name: API Extractor
run: yarn api-extractor:ci
run: pnpm api-extractor:ci
8 changes: 7 additions & 1 deletion .github/workflows/automation.yml
View file
Open in desktop
Expand Up @@ -7,9 +7,15 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2

- name: Install pnpm
uses: pnpm/action-setup@v2
with:
version: latest

- name: Cypress run
uses: cypress-io/github-action@v4
with:
start: npm run start
start: pnpm start
wait-on: 'http://localhost:3000'
wait-on-timeout: 120
32 changes: 24 additions & 8 deletions .github/workflows/build-test.yml
View file
Open in desktop
Expand Up @@ -5,24 +5,40 @@ on: [pull_request]
jobs:
build:
runs-on: ubuntu-latest
strategy:
matrix:
# Do we want to run with multiple versions of node?
node-version: [16]
Comment on lines +8 to +11
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bluebill1049 It is recommended to specify which node version we use. Also, we can test RHF on multiple node versions as well. For now, I just fixed it to node 16, we can change to following in the future if it's needed

Suggested change
strategy:
matrix:
# Do we want to run with multiple versions of node?
node-version: [16]
strategy:
matrix:
# Do we want to run with multiple versions of node?
node-version: [16, 18]

Copy link
Member

Choose a reason for hiding this comment

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

yep, 👍


steps:
- name: Checkout repo
uses: actions/checkout@v2
uses: actions/checkout@v3

- name: Install deps and build (with cache)
uses: bahmutov/npm-install@v1
- name: Setup pnpm
uses: pnpm/action-setup@v2
with:
version: 7

- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node-version }}
cache: 'pnpm'

- name: Install dependencies
run: pnpm install

- name: Lint
run: |
yarn lint
yarn type
pnpm lint
pnpm type
# What is --ci?
- name: Test
run: |
yarn test --ci
yarn test:type
pnpm run test --ci
Copy link
Contributor Author

Choose a reason for hiding this comment

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

pnpm test --ci fails,
pnpm run test --ci passed
Not have much time to check more details.
cc: @bluebill1049

Copy link
Member

Choose a reason for hiding this comment

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

haha that's strange, yea pnpm run test --ci is fine.

pnpm test:type
- name: Bundle watch
run: |
yarn bundlewatch
pnpm bundlewatch
9 changes: 9 additions & 0 deletions .github/workflows/compressed-size.yml
View file
Open in desktop
Expand Up @@ -8,6 +8,15 @@ jobs:

steps:
- uses: actions/checkout@v2
- name: Setup pnpm
uses: pnpm/action-setup@v2
with:
version: 7
- name: Use Node.js 16
uses: actions/setup-node@v3
with:
node-version: 16
cache: 'pnpm'
- uses: preactjs/compressed-size-action@v2
with:
repo-token: '${{ secrets.GITHUB_TOKEN }}'
2 changes: 1 addition & 1 deletion .npmrc
View file
Open in desktop
@@ -1 +1 @@
package-lock=false
auto-install-peers=true
8 changes: 4 additions & 4 deletions app/package.json
View file
Open in desktop
Expand Up @@ -2,7 +2,7 @@
"name": "react-hook-form",
"version": "0.0.0",
"scripts": {
"dev": "vite",
"dev": "vite --force",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bluebill1049 --force is to invalidate the pre-bundle dependencies. This is a workaround. When you make changes to RHF, please restart the dev server of app.
We will try to improve this behavior to have real HMR. (See my comment in app/vite.config.ts for more)

Copy link
Member

Choose a reason for hiding this comment

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

Yea, no worries on that.

"build": "tsc && vite build",
"serve": "vite preview"
},
Expand All @@ -13,7 +13,7 @@
"joi": "^17.5.0",
"react": "^17.0.1",
"react-dom": "^17.0.1",
"react-hook-form": "^7.25.0",
"react-hook-form": "file:..",
Copy link

Choose a reason for hiding this comment

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

@nvh95 Hi, I have question why you don't use workspace:* keyword here?

Copy link

Choose a reason for hiding this comment

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

aha still no pnpm-workspace.yaml file on project root,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @sadeghbarati. I considered that option #9516 (comment)

However, now, I am looking for the simplest solution, since this PR focuses on migrating to pnpm. In the next PR, I will adopt pnpm Workspace and use workspace:*.

Thanks for your suggestion 🤗.

Copy link

Choose a reason for hiding this comment

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

Just saw your Tweet, Thanks for your efforts

"react-router-dom": "^6.2.1",
"react-select": "^5.2.2",
"yup": "^0.32.8"
Expand All @@ -24,8 +24,8 @@
"@types/react-router-dom": "^5.3.3",
"@types/react-select": "^5.0.1",
"@types/yup": "^0.29.13",
"@vitejs/plugin-react": "^1.3.1",
"@vitejs/plugin-react": "^2.2.0",
"typescript": "^4.5.5",
"vite": "^2.9"
"vite": "^3.2.4"
}
}