-
-
Notifications
You must be signed in to change notification settings - Fork 839
refactor: use protected methods instead of computed properties to allow tree shaking
#4458
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
Conversation
…ow `tree shaking`
|
@yusukebe |
Bundle size check
Compiler Diagnostics (tsc)
Compiler Diagnostics (typescript-go)
Reported by octocov |
HTTP Performance Benchmark
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4458 +/- ##
==========================================
+ Coverage 91.28% 91.29% +0.01%
==========================================
Files 173 173
Lines 11064 11067 +3
Branches 3189 3190 +1
==========================================
+ Hits 10100 10104 +4
+ Misses 963 962 -1
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| import type { HandlerData, Matcher, MatcherMap, StaticMap } from './matcher' | ||
| import { match, buildAllMatchersKey, emptyParam } from './matcher' | ||
| import { match, emptyParam } from './matcher' | ||
| import { RegExpRouter } from './router' |
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.
This is not related to this PR, but it uses only the type so that you can write like this:
import type { RegExpRouter } from './router'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.
Thank you for your comment.
We're using it as a base class here!
If bundling doesn't produce the intended result, I think it's possible to separate the files to minimize the impact.
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.
@usualoma Ah, class! Sorry, my misunderstanding. It's okay as it is.
|
I confirmed this change affects the new project and reduces the file size without RegExpRouter. Thanks! I've left a comment. |
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.
LGTM!
|
Looks good! Thank you for the great work. |
The presence of computed properties led to the determination that side effects were possible, resulting in larger bundled code.
file
bundle size
main branch
refactor/prepared-computed-property branch
The author should do the following, if applicable
bun run format:fix && bun run lint:fixto format the code