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

Use webpack IgnorePlugin instead of externals #791

Closed
wants to merge 1 commit into from
Closed

Use webpack IgnorePlugin instead of externals #791

wants to merge 1 commit into from

Conversation

remcohaszing
Copy link

The documentation describes that optional dependencies should be marked as externals in webpack. This means that they will still show up in the source map.

The new setup simply ignores them, which is more correct. This was already reflected by Enzyme's own karma webpack configuration.

The React 0.14 ignore list was also written as a webpack config for consistency.

The documentation describes that optional dependencies should be marked as externals in webpack. This means that they will still show up in the source map.

The new setup simply ignores them, which is more correct. This was already reflected by Enzyme's own karma webpack configuration.

The React 0.14 ignore list was also written as a webpack config for consistency.
@aweary
Copy link
Collaborator

aweary commented Feb 15, 2017

Thanks for the PR! @remcohaszing have you verified that this setup works with Webpack 1/2 in all our supported versions of React?

@remcohaszing
Copy link
Author

I didn't test it with older versions of React, but I don't see why it wouldn't work.

The old docs stated:

you simply need to make sure that the following files are labeled as "external", which means they will be ignored:

By labeling them as "external" they're not really ignored, just undefined. Now they truly are ignored.

/* webpack.config.js */
// ...
plugins: [
new webpack.IgnorePlugin(/cheerio/),
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need to ignore cheerio and react-dom/server? our own karma.conf.js seems to work ok without ignoring these

// ...
plugins: [
new webpack.IgnorePlugin(/cheerio/),
new webpack.IgnorePlugin(/react-dom/),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this list of ignore plugins is actually for react 13 not 14, they're swapped for some reason?

Copy link
Author

Choose a reason for hiding this comment

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

I just converted the list to a list of webpack.IgnorePlugin instances. If the list is wrong, I'd say that's unrelated to this patch. I've only used react 0.15, so I don't know what should be ignored exactly.

@ljharb
Copy link
Member

ljharb commented Sep 26, 2017

I don't think this is relevant any more now that we're on v3.

@ljharb ljharb closed this Sep 26, 2017
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.

5 participants