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: cache dynamic imports #8652

Merged
merged 1 commit into from Jul 7, 2022
Merged

fix: cache dynamic imports #8652

merged 1 commit into from Jul 7, 2022

Conversation

OrKoN
Copy link
Collaborator

@OrKoN OrKoN commented Jul 7, 2022

Closes #8650

@OrKoN OrKoN requested a review from jrandolf July 7, 2022 16:44
@OrKoN OrKoN marked this pull request as ready for review July 7, 2022 16:44
Copy link
Contributor

@jrandolf jrandolf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Node should do this themselves....facepalm

@OrKoN OrKoN merged commit 1de0383 into main Jul 7, 2022
@OrKoN OrKoN deleted the fix-dynamic-imports branch July 7, 2022 19:09
@Kikobeats
Copy link
Contributor

I really will appreciate some explanation how what's happening here; I'm trying to understand how the perf degradation was caused and how this PR resolve the thing, but is very unclear to me; I don't understand why these helpers are even necessary 😅

@prarabdhb
Copy link

@Kikobeats From what I understood, in the previous implementation, fs or debug was imported every time the import statement was encountered. To ensure this doesn't happen, this PR stores fs in a variable fs so that it is not imported every time, improving the performance as importing can be an expensive task.

@Kikobeats
Copy link
Contributor

I still feel very lost since modules (ESM/CJS) are singleton by nature, so it doesn't matter how much times you import them, it will be cached in the first call

@jrandolf
Copy link
Contributor

I still feel very lost since modules (ESM/CJS) are singleton by nature, so it doesn't matter how much times you import them, it will be cached in the first call

This is a reasonable assumption, but sadly this is probably not the case based on the stats.

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.

[Bug]: Dramatic performance drop with ESM since v14.0.0
4 participants