Skip to content
This repository has been archived by the owner on Jul 13, 2021. It is now read-only.

Resolve path to "webpack-hot-client/client" (support for monorepos) #62

Closed
wants to merge 2 commits into from

Conversation

frenzzy
Copy link

@frenzzy frenzzy commented May 22, 2018

  • This is a bugfix
  • This is a new feature
  • This is a code refactor
  • This is a test update
  • This is a typo fix
  • This is a metadata update

For Bugs and Features; did you add new tests?

Should I?

Motivation / Use-Case

Fixes an error which appears if module installed in the separate node_modules directory:

ERROR in multi webpack-hot-client/client?client ./src/app.js
Module not found: Error: Can't resolve 'webpack-hot-client/client?client' in '/my-create-react-app'

Breaking Changes

Additional Info

Fixes an error which appears if module installed in the separate `node_modules` directory:
```sh
ERROR in multi webpack-hot-client/client?client ./src/app.js
Module not found: Error: Can't resolve 'webpack-hot-client/client?client' in '/my-create-react-app'
```
@jsf-clabot
Copy link

jsf-clabot commented May 22, 2018

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented May 22, 2018

Codecov Report

Merging #62 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #62      +/-   ##
==========================================
+ Coverage   99.32%   99.32%   +<.01%     
==========================================
  Files           3        3              
  Lines         148      149       +1     
==========================================
+ Hits          147      148       +1     
  Misses          1        1
Impacted Files Coverage Δ
lib/util.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 276d950...828b3fd. Read the comment docs.

@shellscape
Copy link
Contributor

Thanks for the PR 🍺

This is a tough one. There should never be more than one version of webpack-hot-client installed so this should succeed no matter what. Your monorepo setup should account for this, in theory, and add the proper symlinks to be able to resolve dependencies.

I'm not convinced this change is needed as yet, but I'm open to further discussion.

For Bugs and Features; did you add new tests?
Should I?

According to the PR template, yes:

<!-- Please note that we won't approve your changes if you don't add tests. -->

@shellscape
Copy link
Contributor

Thanks again for your work and opening a PR. Going to pass on this one, however. In monorepos webpack.config.js can contain a resolve-alias that should cover this case, without having to muck with absolute paths in the codebase. e.g.

  resolve: {
    alias: {
      'webpack-hot-client/client': path.resolve(...),
    },
  },

This approach also allows a user to use require.resolve or enhanced-resolve as alternatives, for example. It's far more flexible than the proposed change in the PR.

@shellscape shellscape closed this May 31, 2018
@frenzzy frenzzy deleted the patch-1 branch June 1, 2018 07:36
lehni added a commit to ditojs/dito that referenced this pull request Mar 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants