-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Webpack + SWC Loader #1650
Webpack + SWC Loader #1650
Conversation
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Bundle Stats — desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset (largest 100 files by percent change)
View detailed bundle breakdownAdded
Removed
Bigger
Smaller
Unchanged
|
Bundle Stats — loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset (largest 100 files by percent change)
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller No assets were smaller Unchanged No assets were unchanged |
Really cool. Let me know when you want me to review this. |
@MatissJanis @shall0pass @trevdor This is ready for review :) There seem to be an increase in bundle size due to the switch but build and test execution time has dramatically improved. Aside: It looks like github action is not caching packages? |
Just pushed a small patch for electron app I found while testing this PR. Hope you don't mind. |
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.
Did not spot any significant issue! Amazing job here :)
@@ -40,7 +43,6 @@ | |||
"memoize-one": "^6.0.0", | |||
"pikaday": "1.8.0", | |||
"react": "18.2.0", | |||
"react-app-rewired": "^2.2.1", |
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.
💬 suggestion: react-scripts
can also be removed IMO
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.
Craco still uses react-scripts under the hood I believe: https://craco.js.org/docs/getting-started/
.github/actions/setup/action.yml
Outdated
with: | ||
path: '**/node_modules' | ||
key: yarn-v1-${{ runner.os }}-${{ hashFiles('.nvmrc') }}-${{ hashFiles('**/yarn.lock') }} | ||
cache: yarn |
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.
💬 suggestion: The default cache mechanism does not take into account .nvmrc
file. That's why we created a custom cache key here.
With the proposed change the cache will not be invalidated when we bump the node version in .nvmrc
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.
Oh I see, I'll revert this change
Ok, this broke VRT.. apologies. I reverted my change. Will patch it in a separate PR. |
🤦 it was not my changes.. the VRT in Updating the branch should solve the issue |
This reverts commit 787af19.
4003f49
to
f8d0075
Compare
I have rebased the branch. Feel free to commit your electron fix :) |
* desktopc-client swc-loader * More swc * Jest swc + upgrades * Revert @swc/jest usage for now * SWC minify * Remove setupFilesAfterEnv in package.json as per warning message in CI * Release notes * Minify on CI * swc helpers in loot-core * @swc/jest * Upgrade webpack * Add @swc/core to crdt * Use yarn cache in github actions * Cleanup * Fix electron * Revert "Fix electron" This reverts commit 787af19. * Revert action.yml cache changes --------- Co-authored-by: Matiss Janis Aboltins <[email protected]>
Related to #656