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

React refresh not compatible with @babel/preset-flow #136

Closed
michaelgmiller1 opened this issue Jul 8, 2020 · 6 comments · Fixed by #140
Closed

React refresh not compatible with @babel/preset-flow #136

michaelgmiller1 opened this issue Jul 8, 2020 · 6 comments · Fixed by #140

Comments

@michaelgmiller1
Copy link

If I include @babel/preset-flow in the webpack-dev-server example, webpack fails to build it. If I remove hot reloading, webpack succeeds. I've created a small reproducible test case here:

https://github.com/michaelgmiller1/react-refresh-webpack-plugin/commits/flow-break

I think this is the last remaining issue for us to adopt React Refresh in our app!! Thanks again for all your work fixing my bug reports!

@michaelgmiller1
Copy link
Author

Oh and the error is:


ERROR in ./src/App.jsx
Module build failed (from ./node_modules/babel-loader/lib/index.js):
SyntaxError: /Users/miller/react-refresh-webpack-plugin/examples/webpack-dev-server/src/App.jsx: Unexpected token (19:23)

  17 |
  18 | function App() {
> 19 |   const test = useTest<*>();
     |                        ^
  20 |
  21 |   return (
  22 |     <div>
    at Object._raise (/Users/miller/react-refresh-webpack-plugin/examples/webpack-dev-server/node_modules/@babel/parser/lib/index.js:757:17)
    at Object.raiseWithData (/Users/miller/react-refresh-webpack-plugin/examples/webpack-dev-server/node_modules/@babel/parser/lib/index.js:750:17)
    at Object.raise (/Users/miller/react-refresh-webpack-plugin/examples/webpack-dev-server/node_modules/@babel/parser/lib/index.js:744:17)
    at Object.unexpected (/Users/miller/react-refresh-webpack-plugin/examples/webpack-dev-server/node_modules/@babel/parser/lib/index.js:8834:16)
    at Object.parseExprAtom (/Users/miller/react-refresh-webpack-plugin/examples/webpack-dev-server/node_modules/@babel/parser/lib/index.js:10169:20)
    at Object.parseExprAtom (/Users/miller/react-refresh-webpack-plugin/examples/webpack-dev-server/node_modules/@babel/parser/lib/index.js:4648:20)
    at Object.parseExprSubscripts (/Users/miller/react-refresh-webpack-plugin/examples/webpack-dev-server/node_modules/@babel/parser/lib/index.js:9688:23)
    at Object.parseMaybeUnary (/Users/miller/react-refresh-webpack-plugin/examples/webpack-dev-server/node_modules/@babel/parser/lib/index.js:9668:21)
    at Object.parseExprOpBaseRightExpr (/Users/miller/react-refresh-webpack-plugin/examples/webpack-dev-server/node_modules/@babel/parser/lib/index.js:9631:34)
    at Object.parseExprOpRightExpr (/Users/miller/react-refresh-webpack-plugin/examples/webpack-dev-server/node_modules/@babel/parser/lib/index.js:9624:21)
 @ ./src/index.js 5:0-24 6:41-44
 @ multi ./node_modules/@pmmmwh/react-refresh-webpack-plugin/client/ReactRefreshEntry.js ./node_modules/@pmmmwh/react-refresh-webpack-plugin/client/ErrorOverlayEntry.js ./src/index.js
Child HtmlWebpackCompiler:
     1 asset
    Entrypoint HtmlWebpackPlugin_0 = __child-HtmlWebpackPlugin_0
    [./node_modules/html-webpack-plugin/lib/loader.js!./public/index.html] 530 bytes {HtmlWebpackPlugin_0} [built]
ℹ 「wdm」: Failed to compile.

@brandonpapworth
Copy link

Along these lines, as I was fiddling around with the example, I noticed that the output that is attempting to be parsed does not have the // @flow comment at the top of the file, which will make things break as the flowtype type-stripper won't be triggered. It appears that ReactRefreshWebpackPlugin is stripping it along the way.

If you were to update examples/webpack-dev-server/babel.config.js to have @babel/preset-flow treat all matched files as flowtyped, everything compiles fine as the missing // @flow is no longer an issue.

module.exports = (api) => {
  // This caches the Babel config
  api.cache.using(() => process.env.NODE_ENV);
  return {
    presets: ['@babel/preset-env', '@babel/preset-react', ['@babel/preset-flow', {all: true}]],
    // Applies the react-refresh Babel plugin on non-production modes only
    ...(!api.env('production') && { plugins: ['react-refresh/babel'] }),
  };
};

However, this is not expected functionality, and setting all like this is quite problematic for a large, pre-existing codebase.

@pmmmwh
Copy link
Owner

pmmmwh commented Jul 8, 2020

It appears that ReactRefreshWebpackPlugin is stripping it along the way.

Not really stripping it away, we inject runtime code (with a loader that runs before Babel) at the start of files so the // @flow comment will be offset by a few lines.

We might have to hard-code something along the lines of lines[0].include('@flow') to handle it ☹️

I'll fix it tomorrow.

@brandonpapworth
Copy link

@pmmmwh thanks so much for the attention on this.

Would it be possible to inject that runtime code after top-of-file comment lines/blocks? I'm not trying to come off as a begger by any means, but just thinking of a way to make it too stiff from hard-coded solutions.

@pmmmwh
Copy link
Owner

pmmmwh commented Jul 8, 2020

Would it be possible to inject that runtime code after top-of-file comment lines/blocks?

The issue is that we don't parse the file, so we have no idea if there exist a top-of-file comment.

I'll see if we can work around that without doing string manipulation but it seems difficult at first glance.

@pmmmwh
Copy link
Owner

pmmmwh commented Jul 12, 2020

Fixed in 0.4.0-beta.8.

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 a pull request may close this issue.

3 participants