Skip to content

Conversation

@nicholasio
Copy link
Contributor

@nicholasio nicholasio commented Feb 10, 2023

matching against moduleData.request would allow folks to exclude virtual modules generated by other loaders (like linaria). For my use-case I can now do this:

new ReactRefreshWebpackPlugin({
	exclude: [/node_module/, /outputCssLoader\.js/],
}),

To prevent react-refresh from being injected into CSS virtual modules (generated by linaria).

See #245 (comment) for more context.

@nicholasio nicholasio changed the title match against moduleData.request as well match against moduleData.request Feb 10, 2023
@nicholasio
Copy link
Contributor Author

Looks like this little change broke most of the conformance tests. Looking into it.

@nicholasio nicholasio mentioned this pull request Feb 10, 2023
7 tasks
@nicholasio
Copy link
Contributor Author

@pmmmwh 👋 Any thoughts on this? If this doesn't feel like the right solution I'm happy to submit a separate PR to help address the issue.

Thanks!

Comment on lines +41 to +46
// if matchResource is set it means another loader is
// dynamically generating a module so let's look into moduleData.request
// to give users the opportunity to exclude them from instrumentation
if (moduleData.matchResource && !match(moduleData.request)) {
return moduleData;
}
Copy link
Owner

Choose a reason for hiding this comment

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

I think we can collapse this into the big if statement above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO readability is easier by doing it outside since we only want to match against moduleData.request if moduleData.matchResource is set but maybe my brain is just not working well right now lol

Copy link
Owner

Choose a reason for hiding this comment

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

I guess another way to do this is to break down the if statement and do a bunch of early returns 🤔

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.

2 participants