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

Restore Webpack loader (and use it for TS) #1039

Closed
theKashey opened this issue Aug 1, 2018 · 16 comments
Closed

Restore Webpack loader (and use it for TS) #1039

theKashey opened this issue Aug 1, 2018 · 16 comments
Assignees

Comments

@theKashey
Copy link
Collaborator

  • React-Hot-Loader v1 uses webpack loader
  • React-Hot-Loader v3 uses webpack loader
  • But React-Hot-Loader v4 removes it, as long, from one point of view it was not needed anymore, and from another - we need babel plugin to properly work in the new ES6 world.

Then we have invented "cold API", and got reasons to get webpack loader back - just to register variables in node_modules as fast as possible.

Then we started thinking about better TypeScript support, and it's too many variants, and all are not "better" - as long they are about replacing TS by babel 7 or running babel before or after TS.

All those variants are or slow, or unacceptable by "hey, it's babel free zone here!" reasons.

Idea:

  • restore webpack loader. To power up cold API.
  • inside webpack loader perform just a simple string search - check for class keyword used. If it will be found - inject RHL methods
  • injecting could be done or via simple string insertion, or we could call babel for a single file.

Result:

  • webpack loader, we can use on node_modules without speed penalties
  • webpack loader, we can use with TS, without speed penalties and complex configuration.
  • probably that should be different loaders, as long one of them will bring(or not) babel with it.
@UselessPickles
Copy link

Is there any estimate for when this will be done? I am currently trying to upgrade a project from Webpack 3 to Webpack 4 (and React Hot Loader 3 -> 4). This new requirement to use Babel for TS is a major hurdle because Babel has some limitations with processing TS.

Specifically, Babel does not support namespaces, and this project uses namespaces. Changing the project to not use namespaces is not practical. Therefore, I simply cannot use Babel to process my Typescript code, and I cannot use React Hot Loader :(

Even if there wasn't this issue with namespaces, it's quite obnoxious to setup babel to process Typescript for the sole purpose of React Hot Loader when nothing else about this project requires Babel.

@Bnaya
Copy link
Contributor

Bnaya commented Sep 4, 2018

You can always put babel loader after ts-loader
That what i do

@sibelius
Copy link

sibelius commented Sep 4, 2018

I prefer to use babel 7 to compile typescript

@gregberge
Copy link
Collaborator

@theKashey with Babel 7 + Typescript, I think it is not a good idea. Better to keep it simple.

@theKashey
Copy link
Collaborator Author

I am seeing webpack loader as a good help for cold api in terms of freezing node_modules.
Meanwhile, I am already working on this (just pushed current state), and look like it's not a quite helpfull - it "registers" only module exports, and some stuff, like StyledComponent with class constructed inside a function, could not be handled by webpack loader.

Meanwhile - we failed to migrate to babel 7 in our own work projects, and still stuck with TS-loader, waiting for all peers to update first (even RHL itself is not babel7 compatible yet).

So

Will do the job

theKashey added a commit that referenced this issue Sep 5, 2018
@UselessPickles
Copy link

While waiting for progress on this issue, I've been trying to get webpack 4 + typescript + react-hot-loader working in my project by using babel with minimal configuration to process the output of ts-loader. So the loaders for my .ts/.tsx files looks a bit like this:

{
    use: [
        {
            loader: "babel-loader",
            options: {
                cacheDirectory: true,
                babelrc: false,
                plugins: ["react-hot-loader/babel"]
            }
        },
        {
            loader: "ts-loader",
            options: {
                transpileOnly: true,
                compilerOptions: {
                    sourceMap: isSourceMap,
                    isolatedModules: true,
                    noEmitOnError: false
                }
            }
        }
    ]
}

But I can't get HMR to work for react components.

After enabling debug logging, I see output like this in the console:

react-hot-loader.development.js?c2cb:181 React-hot-loader: unable to merge (3) [{…}, {…}, {…}] and children of {type: "div", children: Array(2), instance: {…}}
react-hot-loader.development.js?c2cb:181 in Component (6) [{…}, {…}, {…}, {…}, {…}, {…}]

So clearly, react-hot-loader is trying to apply the hot updates, but can't for some reason (the reason is not specified in plain human readable text, and I don't know how to analyze the complex structures in the logs to determine the reason).

I am using the hot HOC for the top-level "App" component of my application, and the hot() version of the component is the one that is exported and rendered.

My suspicion is that my problem is caused by the use of 3rd party react components from node_modules that are NOT processed through babel (and therefore not processed by react-hot-loader). Do I have to process all node_modules code (or at least all react component code) through the babel plugin to get HMR working properly?

@UselessPickles
Copy link

Nevermind; I setup my project to run ALL node_modules code through babel, and I still have the problem. So my problem is unrelated to this issue. I'll create a new issue if I can narrow it down to something more specific than, "I can't get HMR to work" (it seems to be a limitation/bug related to some 3rd party react components; some hot react updates DO work just fine, but updates to my components that are presented within a BlueprintJS Dialog do not hot update correctly, for example).

@theKashey
Copy link
Collaborator Author

Could be React 15 portals. They are breaking normal update processing, but should still be more or less good.
Things to try - decipher error messages. RHL is showing what it should be rendered, and the really. Try to find such place in your application.
(The last part is breadcrumbs - so just 6 elements from the top)

@UselessPickles
Copy link

Could be React 15 portals. They are breaking normal update processing

That seems likely. I only seem to encounter this problem when making code changes to a component that is rendered within a component that uses a portal.

Are there any feasible plans to make react-hot-loader work better with Portals, or is this an unfortunate limitation we're stuck with forever?

And back on topic... any news on reimplementing a webpack loader so that Babel is not required?

@theKashey
Copy link
Collaborator Author

Portals would be fixed by reactjs/rfcs#74, if it ever got implemented.

webpack stuff was more or less finished a month ago, but not yet merged.

@theKashey
Copy link
Collaborator Author

4.6.0 with webpack and controlled type comparison went out

@UselessPickles
Copy link

UselessPickles commented Dec 13, 2018

Could you clarify how the webpack plugin "is not compatible with class-based components"? (quote from the README section about the webpack plugin)

I assume a "class-based component" is any component that is a class that extends React.Component?

What incorrect behavior will I experience with the webpack plugin if my project uses class-based components?

@theKashey
Copy link
Collaborator Author

Babel plugin would inject special member to update class methods, while webpack would do nothing.

So - using only webpack plugin you will be unable to update non-prototype-based methods, especally the arrow functions, or any other "bind"ed callbacks.

class MyComponent extends React.Component {
 variable=1;// this can
 onClick = () => this.setState() // this can NOT

 render() { // this can
}

@UselessPickles
Copy link

Thanks for the explanation. I think it would be helpful to add an explanation like this to the README.

@theKashey
Copy link
Collaborator Author

Mee to :)

@theKashey
Copy link
Collaborator Author

Webpack-loader is restored.

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

No branches or pull requests

5 participants