-
-
Notifications
You must be signed in to change notification settings - Fork 143
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
v3: Migrate packages to monorepo #318
Conversation
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## v3 #318 +/- ##
============================================
- Coverage 100.00% 0.00% -100.00%
============================================
Files 6 1 -5
Lines 228 6 -222
Branches 56 2 -54
============================================
- Hits 228 0 -228
- Misses 0 4 +4
- Partials 0 2 +2 ☔ View full report in Codecov by Sentry. |
"./react-deps.js" | ||
] | ||
"path": "packages/wouter/dist/index.js", | ||
"limit": "2000 B", |
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.
Limits are increased, may I ask why?
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.
to make size-limit
happy)
I included react-deps.js
to bundle-size test and it not only increases the size but causes some deoptimizations i guess:
wouter/packages/wouter/src/react-deps.js
Lines 4 to 9 in e5544ca
const { useEffect, useLayoutEffect, useRef, useInsertionEffect: useBuiltinInsertionEffect, } = React;
this code can be deleted and included in final size tests
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.
it should be
import {
useEffect,
useLayoutEffect,
useRef,
useInsertionEffect as useBuiltinInsertionEffect,
} from 'react';
to fix all three-shaky stuff
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.
but it can break some old react versions that doesn't provide useInsertionEffect
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.
In the root, there is a file paths.js
I guess we don't need it anymore.
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.
In the root, there is a file paths.js
I guess we don't need it anymore.
wouter
andwouter-preact
to monorepoesm
, dropcjs
exportsjest
(but coverage is broken)