Skip to content
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 injectRefreshLoader performance issue #669

Merged
merged 2 commits into from Sep 26, 2022
Merged

Conversation

marco2216
Copy link
Contributor

@marco2216 marco2216 commented Sep 1, 2022

After investigating some performance issues that only appear when enabling this plugin, I discovered that the require.resolve('react-refresh') which gets called in injectRefreshLoader seems to be the cause.
Moving the call outside the injectRefreshLoader cuts initial compile time in more than half (~40s -> ~15s) and incremental compile time has a similar speedup (~8s -> ~3s).

I don't know why, but I guess the require.resolve is not cached for some reason, or just takes some time, which adds up when the function runs for thousands of modules?

The change moving the refreshUtilsPath outside the function is not required to improve the performance, but I don't see why it wouldn't be moved outside the function as it should be static.

Tested on two different machines running the same project. May or may not address this issue: #543

@marco2216
Copy link
Contributor Author

@pmmmwh Please let me know if you need any additional information to get this merged!

@pmmmwh
Copy link
Owner

pmmmwh commented Sep 26, 2022

Thanks!

@pmmmwh pmmmwh merged commit e0f8a42 into pmmmwh:main Sep 26, 2022
@chenjiahan
Copy link

@pmmmwh Can you release a new version for this change?

Thank!

@JSerFeng
Copy link

Any idea on why require.resolve is not cached 🤔

@pmmmwh
Copy link
Owner

pmmmwh commented Oct 9, 2022

Released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants