-
-
Notifications
You must be signed in to change notification settings - Fork 5k
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
refactor: generate routes and template files in builder #4883
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #4883 +/- ##
==========================================
- Coverage 91.89% 91.74% -0.15%
==========================================
Files 72 73 +1
Lines 2406 2412 +6
Branches 593 593
==========================================
+ Hits 2211 2213 +2
- Misses 177 181 +4
Partials 18 18
Continue to review full report at Codecov.
|
: null | ||
} | ||
} | ||
const templateContext = new TemplateContext(this, this.options) |
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.
Can we refactor rest of this function to a TemplateBuilder
class?
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.
I tried, but the methods are all couple with builder's members and methods like resolveRelativeToBuild
, supportedExtensions
, template
, routes
, watch
....
So I reverted my changed for avoiding leak builder
into other class.
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.
LSexyTM at this point :D
'' | ||
) | ||
|
||
if (p.ssr === false) { |
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.
We could refactor that block into a dedicated function
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.
Do you mean marshalling mode
in normalizePlugins
?
I think current method is not so complex and the marshalling is not shareable as well, if it can be shared, we can move it into some common util class then.
Types of changes
Description
Checklist: