-
-
Notifications
You must be signed in to change notification settings - Fork 212
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
fix(perf): avoid using klona
for postcss options
#658
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #658 +/- ##
==========================================
- Coverage 88.76% 88.70% -0.07%
==========================================
Files 3 3
Lines 356 354 -2
Branches 115 115
==========================================
- Hits 316 314 -2
Misses 37 37
Partials 3 3
☔ View full report in Codecov by Sentry. |
@@ -185,11 +184,9 @@ async function loadConfig(loaderContext, config, postcssOptions) { | |||
options: postcssOptions || {}, | |||
}; | |||
|
|||
result.config = result.config(api); | |||
return { ...result, config: result.config(api) }; |
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.
Very uninformed questions, but this is not the same as a deep clone :
- Will it not create issue ?
- Was a deep clone here an overkill in the first place ?
Thank you for your time ! You are a pillar of the Webpack ecosystem !
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.
No, it is not deep clone, but here result
, contains only path
, so we can use to use ...
, just to avoid mutable result
Will it not create issue ?
Was a deep clone here an overkill in the first place ?
As I said above, there is no deep only, here the config
property from require
and path
(it just a string)
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.
Ok thank you so mush for the details and your time. Have a nice day !
This PR contains a:
Motivation / Use-Case
Do not use
klona
- better perf and less depsBreaking Changes
No
Additional Info
No