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

Create explicit loader path #555

Closed
wants to merge 1 commit into from
Closed

Create explicit loader path #555

wants to merge 1 commit into from

Conversation

ovidiuch
Copy link

Problem

The html-webpack-plugin/lib/loader.js path assumes html-webpack-plugin will be found where webpack runs, except in some rare cases it isn't so. In Lerna setups like that of react-cosmos, you can have one package have html-webpack-plugin as a dep and another package have webpack as a dep. Because the way packages are linked in this setup they each have their own node_modules bucket and deps installed on one side aren't found in the other (until lib is actually published and user installs, then everything ends up normalized in the same node_modules dir).

Solution

By pinning down the exact path this problem disappears. Can't think of any downside and all tests pass. Thoughts?

Thanks!

@jantimon
Copy link
Owner

jantimon commented Jan 22, 2017

It used to be require.resolve instead of __dirname before but it was changed because of the ugly webpack output

v2.24.1...v2.25#diff-168726dbe96b3ce427e7fedce31bb0bcL596

@ovidiuch
Copy link
Author

Either require.resolve or __dirname works for me. Maybe we can strip the root path on output?

@jantimon
Copy link
Owner

The output is handled by webpack itself.

The root of the problem is the webpack loader resolve logic - How would react-comos work with loaders?

@ovidiuch
Copy link
Author

How would react-comos work with loaders?

All loader paths are resolved, for webpack to load them from the inside of that particular package (i.e. react-cosmos-webpack) and not resolved from where the webpack build context is at (i.e. package depending on react-cosmos-webpack). As prev said, usually all deps are in the same place, expect for cases like npm link or Lerna.

@jantimon
Copy link
Owner

What about this one?

Webpack 1:

resolveLoader: {
  fallback: path.join(__dirname, '../node_modules'),
},

And for Webpack 2:

resolveLoader: {
  modules: [
    'node_modules',
    path.join(__dirname, '../node_modules'),
  ],
},

@ovidiuch
Copy link
Author

What about this one?

Should work, but react-cosmos currently supports both webpack 1 & 2, so it would require some wizardry and create the config based on the detected version.

I switched to using a fork that includes this PR to move forward. Not happy with it because it adds to the management cost, but feel free to close this if you're sure you don't want to return to resolving the loader path.

@jantimon
Copy link
Owner

I guess I will revert the change so it works again with plugins like yours again and move the change to html-webpack-html 3.x version which won't support webpack 1 anyway

@jantimon
Copy link
Owner

Fixed in 2.27.1

@jantimon jantimon closed this Jan 29, 2017
@lock
Copy link

lock bot commented May 31, 2018

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants