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

nx and rspack on existing project structure #805

Closed
JohnTParsons opened this issue Apr 4, 2023 · 3 comments · Fixed by #847
Closed

nx and rspack on existing project structure #805

JohnTParsons opened this issue Apr 4, 2023 · 3 comments · Fixed by #847
Assignees
Labels
feature This change contains a new feature

Comments

@JohnTParsons
Copy link
Collaborator

JohnTParsons commented Apr 4, 2023

Purpose of the spike

Spike nx and rspack on existing project structure

##Steps:

  1. Apply "npx add-nx-to-monorepo" to newskit repo (will work even though it is not a mono repo). It will show a series of prompts. Select test and lint tasks to upgrad, this will convert them from "yarn ..." to "nx exec..."
  2. add rspack to project
  3. change the lint and test scripts in package.json from "nx exec" to "nx rspack"
  4. any other changes required for it to work well

Outcomes of the Spike

Faster lint and test

Time box

2 days

Supporting Information

Process/sequence flows, wireframes, third party documentation, tech feasibility, architecture overview/documentation, designs, data specs etc...

@JohnTParsons JohnTParsons added triage This issue will be reviewed by the team spike labels Apr 4, 2023
@GeriReid GeriReid removed the triage This issue will be reviewed by the team label Apr 4, 2023
@GeriReid GeriReid moved this to Analysis in NewsKit Apr 4, 2023
@jps jps added this to the Infrastructure & Release milestone Apr 13, 2023
@JohnTParsons JohnTParsons moved this from Analysis to Ready for estimation in NewsKit Apr 20, 2023
@jps jps moved this from Ready for estimation to Estimated (not prioritised) in NewsKit Apr 20, 2023
@JohnTParsons JohnTParsons moved this from Estimated (not prioritised) to Ready to do in NewsKit Apr 21, 2023
@JohnTParsons JohnTParsons self-assigned this Apr 21, 2023
@JohnTParsons JohnTParsons moved this from Ready to do to In progress in NewsKit Apr 21, 2023
@JohnTParsons JohnTParsons changed the title Spike nx and rspack on existing project structure nx and rspack on existing project structure Apr 24, 2023
@JohnTParsons JohnTParsons added feature This change contains a new feature and removed spike labels Apr 24, 2023
@JohnTParsons
Copy link
Collaborator Author

Test results on pipeline:

lint:
Lint step:
before: 305s
after: 34s
improvement: 9x quicker

test_unit_comps:
Unit Testing step:
before: 120s
after: about the same, as newer techniques were much slower on pipeline, so had to force pipeline to use the old "node jest with garbage collection" technique

test_unit_docs:
Unit Testing step:
before: 177s
after: about the same, as newer techniques were much slower on pipeline, so had to force pipeline to use the old "node jest with garbage collection" technique

@JohnTParsons
Copy link
Collaborator Author

Test results running locally (entering package.json commands on the terminal).

Both lint and test is noticeably quicker than before.

yarn clear:cache

NX Resetting the Nx workspace cache and stopping the Nx Daemon.
This might take a few minutes.
NX Daemon Server - Stopped
NX Successfully reset the Nx workspace.
✨ Done in 0.50s.

yarn lint:code:comps --skip-nx-cache
✖ 17 problems (0 errors, 17 warnings)

NX Successfully ran target lint for project newskit-ui (12s)
✨ Done in 14.24s.

yarn lint:code:comps
✖ 17 problems (0 errors, 17 warnings)

NX Successfully ran target lint for project newskit-ui (13s)
✨ Done in 13.30s.

yarn lint:code:comps
✖ 17 problems (0 errors, 17 warnings)

NX Successfully ran target lint for project newskit-ui (13s)
✨ Done in 13.74s.
PASS - ALTHOUGH NO CACHING TOOK PLACE

yarn lint:code:docs --skip-nx-cache
✖ 7 problems (0 errors, 7 warnings)

NX Successfully ran target lint for project site (7s)
✨ Done in 7.60s.

6a.
yarn lint:code:docs
✖ 7 problems (0 errors, 7 warnings)

NX Successfully ran target lint for project site (7s)
✨ Done in 7.62s.

6b.
yarn lint:code:docs
✖ 7 problems (0 errors, 7 warnings)

NX Successfully ran target lint for project site (7s)
✨ Done in 7.62s.
PASS - ALTHOUGH NO CACHING TOOK PLACE

yarn lint:code --skip-nx-cache
[site] > NX Successfully ran target lint for project site
[comps] > NX Successfully ran target lint for project newskit-ui
✨ Done in 14.14s.

8a.
yarn lint:code
[site] > NX Successfully ran target lint for project site
[comps] > NX Successfully ran target lint for project newskit-ui
✨ Done in 14.08s.

8b.
yarn lint:code
[site] > NX Successfully ran target lint for project site
[comps] > NX Successfully ran target lint for project newskit-ui
✨ Done in 14.20s.
PASS - ALTHOUGH NO CACHING TOOK PLACE

yarn lint
[markdown] > NX Successfully ran target lint:markdown for project newskit
[next] > NX Successfully ran target lint:next for project newskit
[docs] > NX Successfully ran target lint for project site
[comps] > NX Successfully ran target lint for project newskit-ui
✨ Done in 15.12s.

yarn test:unit:comps --skip-nx-cache

NX Successfully ran target test:unit:comps for project newskit (40s)
✨ Done in 40.84s.
(then✨ Done in 26.91s.)

yarn test:unit:comps:experimental --skip-nx-cache
✨ Done in 34.96s.
(then ✨ Done in 20.76s.)

yarn test:unit:comps

NX Successfully ran target test:unit:comps for project newskit (79ms)
Nx read the output from the cache instead of running the command for 1 out of 1 tasks.
✨ Done in 1.69s.
CACHING TOOK PLACE

yarn test:unit:docs --skip-nx-cache

NX Successfully ran target test:unit:docs for project newskit (33s)
✨ Done in 37.74s.

yarn test:unit:docs:experimental --skip-nx-cache

NX Successfully ran target test for project site (1m)
✨ Done in 65.75s.
(then ✨ Done in 26.09s.)

15a.
yarn test:unit:docs

NX Successfully ran target test:unit:run for project newskit (28s)
✨ Done in 29.67s.

15b.
yarn test:unit:docs

NX Successfully ran target test:unit:run for project newskit (64ms)
Nx read the output from the cache instead of running the command for 1 out of 1 tasks.
✨ Done in 1.49s.
CACHING TOOK PLACE

yarn test --skip-nx-cache

NX Successfully ran target test:unit:dev for project newskit (3m)
✨ Done in 73.61s.

17a.
yarn test
[markdown] > NX Successfully ran target lint:markdown for project newskit
[next] > NX Successfully ran target lint:next for project newskit
[docs] > NX Successfully ran target lint for project site
[comps] > NX Successfully ran target lint for project newskit-ui

NX Successfully ran target test:unit:dev for project newskit (1m)
✨ Done in 24.45s.

18b.
yarn test
✨ Done in 23.27s.

yarn test:unit:watch
WORKS AFTER REVERTING IT TO THE PRE-NX WAY OF RUNNING JEST

yarn build
✨ Done in 120.19s.

@JohnTParsons JohnTParsons moved this from In progress to Peer review in NewsKit Apr 27, 2023
@JohnTParsons
Copy link
Collaborator Author

JohnTParsons commented Apr 28, 2023

My performance gains when running locally:

yarn lint --skip-nx-cache
before: 81s
after: 16s
improvement: 5x

Test without coverage would be much quicker than this:

yarn test:unit:run:local --skip-nx-cache
before: 167s
after: 49s
improvement: 3.4x

@JohnTParsons JohnTParsons moved this from Peer review to Done in NewsKit Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This change contains a new feature
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants