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.

Sign up for GitHub

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

Jump to bottom

feat: set react-refresh globals on Webpack require instead of global scope #102

Merged
merged 16 commits into from
Jun 8, 2020
Merged

feat: set react-refresh globals on Webpack require instead of global scope #102

merged 16 commits into from
Jun 8, 2020

Conversation

pmmmwh
Copy link
Owner

@pmmmwh pmmmwh commented May 29, 2020

This PR moves all react-refresh related globals to Webpack's require function instead of renderers' global scope.

This eliminates issues related to:

  • Renderers that do not have window defined
  • Electron/WebWorker/Native-related usage
  • Multiple bundles included for the same page

Fixes #88.
Related to #36 and #79.

CC @sokra - if possible please take a look at this (possible issues/optimizations?)

@pmmmwh pmmmwh added the enhancement New feature or request label May 29, 2020
@pmmmwh pmmmwh self-assigned this May 29, 2020
@pmmmwh pmmmwh force-pushed the feat/parser-hooks branch from e080cc2 to 3d30d31 Compare May 29, 2020 09:46
@pmmmwh pmmmwh force-pushed the feat/parser-hooks branch from 3d30d31 to 6f0fa55 Compare May 29, 2020 15:42
@michaelgmiller1
Copy link

Very excited for this!! If you need any help testing, happy to lend a hand!

sokra
sokra approved these changes Jun 6, 2020
View reviewed changes
Copy link

@sokra sokra left a comment

Choose a reason for hiding this comment

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

The webpack parts look good to me

@shirakaba shirakaba mentioned this pull request Jun 7, 2020
Closed
@pmmmwh pmmmwh merged commit a01c354 into master Jun 8, 2020
@pmmmwh pmmmwh deleted the feat/parser-hooks branch June 8, 2020 06:29
@pmmmwh pmmmwh mentioned this pull request Jun 8, 2020
Closed
@manelio
Copy link

manelio commented Aug 11, 2020

About the point

Multiple bundles included for the same page

That fixes #88:

I modified the webpack-dev-server example this way:

webpack.config.js:

...
  entry: {
    main1: './src/index1.js',
    main2: './src/index2.js',
  },
...

public/index.html:

...
  <body>
    <div id="app1"></div>
    <div id="app2"></div>
  </body>
...

src/index1.js:

import * as React from 'react';
import { render } from 'react-dom';
import App1 from './App1';
render(<App1 />, document.getElementById('app1'));

src/App1.jsx:

import * as React from 'react';

function App1() {
  return <h1>Test1</h1>;
}

export default App1;

And similarly for index2.js and App2.jsx.

But the behavior is, if I made a change in App1.jsx, the DOM is updated, but nothing happens if I modify App2.jsx.

What does it mean "Multiple bundles included for the same page"? Is not this case?

@pmmmwh
Copy link
Owner Author

pmmmwh commented Aug 12, 2020

What does it mean "Multiple bundles included for the same page"? Is not this case?

Sorry I cannot dig into this right now, this might be a regression somewhere.

Can you try make both bundles share the same instance of React and React DOM and see if it works?

@pmmmwh
Copy link
Owner Author

pmmmwh commented Aug 12, 2020

@manelio

Just add this to your Webpack config, it should work:

module.exports = {
  // ... Other stuff
  optimization: {
    runtimeChunk: 'single',
  },
};

@vaishnavi1029
Copy link

TypeError: $RefreshSig$ is not a function
I am writing tests and getting these error this when I am trying to check before and after context values change.
can you let me know why is this happening and how should I potentially eliminate it

@pmmmwh
Copy link
Owner Author

pmmmwh commented Apr 25, 2024

You probably shouldn't apply react-refresh/babel when running tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sokra sokra sokra approved these changes

Assignees

@pmmmwh pmmmwh

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Multiple entry points not working correctly
5 participants
@pmmmwh @michaelgmiller1 @manelio @vaishnavi1029 @sokra