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

Keep array-based entries relative-order during injectRefreshEntry #165

Conversation

miguel-silva
Copy link
Contributor

First of all: hi @pmmmwh and thanks a lot for your work in making it easy to bring react-refresh to webpack based projects. 👏

Motivation

In some of the projects I am currently working on, the bundles are loaded from a regional CDN, and thus we need to rely on a dynamic webpack public path.

// in set-env.js file
__webpack_public_path__ = `https://${cdnDomain}/my-assets-path/`;

I found that, for HMR - in this case based on webpack-hot-middleware - to work appropriately with a dynamic webpack public path one has to inject an entry that defines the dynamic public path prior to the socket entry:

// in webpack.config.js
...
  entry: {
    app: [
      './src/set-env.js',
      'webpack-hot-middleware/client?path=__webpack_hmr&dynamicPublicPath=true',
      './src/app.js'
    ]
  }
...

When I added react-refresh-webpack-plugin (based on the webpack-hot-middleware example) I noticed that it changed the relative order of the entries, outputting something like this:

// entries output (before this PR)
[
  ReactRefreshEntry,
  'webpack-hot-middleware/client?path=__webpack_hmr&dynamicPublicPath=true',
  ErrorOverlayEntry,
  './src/set-env.js',
  './src/app.js'
]

This broke the HMR setup because the dynamic public path definition no longer is set before the socket entry.

Changes

Ensure that injectRefreshEntry keeps the relative order of an array-based entry. Given the example above:

// entries output (after this PR)
[
  ReactRefreshEntry,
+ './src/set-env.js',
  'webpack-hot-middleware/client?path=__webpack_hmr&dynamicPublicPath=true',
  ErrorOverlayEntry,
- './src/set-env.js',
  './src/app.js'
]

Let me know if there are doubts about the problem and/or solution, and feel free to drop any suggestions.

Thank you for your time!

@miguel-silva miguel-silva changed the title Keep relative array entries order during injectRefreshEntry Keep array-based entries order during injectRefreshEntry Jul 26, 2020
@miguel-silva miguel-silva changed the title Keep array-based entries order during injectRefreshEntry Keep array-based entries relative-order during injectRefreshEntry Jul 26, 2020
@pmmmwh pmmmwh merged commit c8a8369 into pmmmwh:main Jul 27, 2020
@pmmmwh
Copy link
Owner

pmmmwh commented Jul 27, 2020

Good catch, thanks!

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