Fix: resolve performance issue due to webpack stats #1391
Conversation
| @@ -357,7 +357,7 @@ export const DevServerMixin = { | |||
| _getWebpackAsset(req, compilerName, fileName) { | |||
| if (req.app.__webpackReady()) { | |||
| const outputFileSystem = req.app.__devMiddleware.context.outputFileSystem | |||
| const jsonWebpackStats = req.app.__devMiddleware.context.stats.toJson() | |||
| const jsonWebpackStats = req.app.__devMiddleware.context.stats.toJson({preset: 'none', outputPath: true}) | |||
There was a problem hiding this comment.
What does this change do? How does it impact people who aren't seeing excessive slowdowns? Is there anything we can test for to validate these changes work as expected? (Not blocking, just questions.)
There was a problem hiding this comment.
@wjhsf what stats.toJson does is essentially getting the stats data in JSON format, and you have the option to selectively query the piece of data you want/need. If we don't pass in the option, by default, you'll get all of the data. Depending on the project, you may get a lot of data.
And so the conversion process to JSON may run slowly if there's a lot of data to generate.
In our function, we only need 1 piece of data outputPath. Hence, we now pass in the option {preset: 'none', outputPath: true}:
- start off with
nonewhich is no data at all - and then add the
outputPath
That option will not affect people who are not seeing the slowdowns. But will be beneficial to projects who have huge stats data.
There was a problem hiding this comment.
I'm curious about the performance gain for this change for the OOTB retail react app. How much time are we saving in ballpark?
There was a problem hiding this comment.
btw, it would be nice to add a comment to explain the "why"
@vmarta We're planning to do a preview patch release, and once the customer confirms that this issue has been resolved, we can do a full patch release. If I understand correctly, this PR will need to be merged in first and we'll have a separate PR for version bump EDIT: re-read your comment, going to bump version in this PR |
Description
Types of Changes
Changes
How to Test-Drive This PR
Checklists
General
Accessibility Compliance
You must check off all items in one of the follow two lists:
or...
Localization