What about react HMR

[testerez]

Sucrase looks promising for a faster dev experience. But what about support form HMR with react?
The popular solution is to use react-hot-loader but it's a babel plugin... Any existing solution to support this feature with Sucrase?

[alangpierce]

Hmm, this is interesting. At my work, we use react-hot-loader 1.x, which I believe didn't have a Babel plugin. Even with the newest version, my reading of things is that the Babel plugin helps with some edge cases, but isn't necessary for most hot reloading. (I haven't tried it yet, though.)

I'll take a closer look at this when I get a chance. I agree that hot reloading is important enough that Sucrase should have some answer to it. I'm certainly not opposed to implementing it directly in Sucrase, although I worry about the implementation being really difficult, depending on what the Babel plugin really needs to do.

[kevinbarabash]

The current version of react-hot-loader uses an HOC proxy component which I think should be usable with sucrase with a some tweaking.

[iki]

The new react-hot-loader@4 uses babel with typescript. We're using it with webpack4 and ts-loader, but didn't test it with sucrase yet, as we're blocked with #225. Anyway the babel step will slow things down, so direct sucrase support would be great.

 // Load TypeScript (.ts, .tsx).
      {
        test: /\.tsx?$/,
        use: [
          ...(isDevServer
            ? [
                {
                  loader: 'babel-loader',
                  options: {
                    babelrc: false,
                    plugins: ['react-hot-loader/babel'],
                  },
                },
              ]
            : []),
          ...(targetES6
            ? [
                {
                  loader: '@sucrase/webpack-loader',
                  options: {
                    transforms: ['imports', 'jsx', 'typescript'],
                  },
                },
              ]
            : [{ loader: 'ts-loader', options: { configFile: paths.tsConfig } }]),
        ],
      },

[theKashey]

G'Day from React-Hot-Loader.
We need babel plugin for 2 things.

1. "register" variables, to undertand their roots. Just scans top level variables, and calls .register method. This is important step, but could be skipped completely.

Could be replaces by webpack loader.

2. Injects "regenerate" function with eval inside to update arrow component properties inside the "scope" of a component. Without it you will not be able to update non-prototype based methods (like onClick).

We also going to replace it by something, but that something did not exists yet. Probably - nothing, but we have to change React first.

It should be super hard to create a sucrase plugin to do the same.
We would be happy to switch to sucrase for TS and node_modules transpiling, but "Sucrase is not pluginizable."?

[alangpierce]

Thanks for the details!

Sucrase is not pluginizable

Yeah, maybe in the future there will be a reasonable plugin API, but for now I'm pretty hesitant about that. Sucrase's primary goal is to be fast, and to do that, it drops a bunch of parser features (keeping only what the transforms really need) and does a single replacement pass through the code, which is less composable than Babel's approach. In practice, adding new transforms has required me to make changes to the transform structure so that they cooperate well. Babel's parser is similarly non-extensible: https://new.babeljs.io/docs/en/next/babel-parser.html#faq

react-hot-loader is popular enough that I think it's fine for the transform to be built-in. (For less popular things, I think the best approach would be a fork.) I suppose that makes it harder to update the transform, though, so maybe there's some partial plugin API that would allow basic details of the react-hot-loader transform to be updated independently.

I looked at some examples and read the code for the Babel plugin. Here's an example:

import React from 'react';

const x = 3;

function blah() {
}

class Foo extends React.Component {
  _handleSomething = () => {
    return 3;
  };

  render() {
    return <span onChange={this._handleSomething} />;
  }
}

class SomeOtherClass {
  test() {}
}

export default 12;

becomes:

(function () {
  var enterModule = require('react-hot-loader').enterModule;

  enterModule && enterModule(module);
})();

import React from 'react';

const x = 3;

function blah() {}

class Foo extends React.Component {
  _handleSomething = () => {
    return 3;
  };

  render() {
    return <span onChange={this._handleSomething} />;
  }

  // @ts-ignore
  __reactstandin__regenerateByEval(key, code) {
    // @ts-ignore
    this[key] = eval(code);
  }

}

class SomeOtherClass {
  test() {}

  // @ts-ignore
  __reactstandin__regenerateByEval(key, code) {
    // @ts-ignore
    this[key] = eval(code);
  }

}

const _default = 12;
export default _default;
;

(function () {
  var reactHotLoader = require('react-hot-loader').default;

  var leaveModule = require('react-hot-loader').leaveModule;

  if (!reactHotLoader) {
    return;
  }

  reactHotLoader.register(x, 'x', 'test.js');
  reactHotLoader.register(blah, 'blah', 'test.js');
  reactHotLoader.register(Foo, 'Foo', 'test.js');
  reactHotLoader.register(SomeOtherClass, 'SomeOtherClass', 'test.js');
  reactHotLoader.register(_default, 'default', 'test.js');
  leaveModule(module);
})();

;

It looks like there are a few pieces here:

• Identify all top-level variables in the file. Sucrase doesn't do this right now, but I think there should be a way to make it work. The parser might need to give a little bit more information, though. This is probably the hardest part.
• Generate code snippets at the start and end of the file based on those top-level variables. This should be pretty easy.
• For every class that has at least one non-static method and does not already have a __reactstandin__regenerateByEval method, add a __reactstandin__regenerateByEval method. It seems like it should be fine to instead just add the method unconditionally, which would be simpler, but let me know if that would cause problems. There's already a place where we insert code at the start of every class body (to implement class fields for classes without constructors), so I think this should be reasonable.
• If there is an export default [expression] statement, extract it into a temp variable and add it as another binding. This will probably need special cases for when the imports transform is enabled/disabled, but I think should be possible.

Hopefully I can get this working reasonably soon. I'll probably have some time this weekend.

[theKashey]

• Identify all top-level variables in the file. Sucrase doesn't do this right now, but I think there should be a way to make it work. The parser might need to give a little bit more information, though. This is probably the hardest part.

we just have to know "variables" origin. This partially could be made using webpack-loader

• Generate code snippets at the start and end of the file based on those top-level variables. This should be pretty easy.

especially for webpack-loader :). And "the top level" block is not required.

• For every class that has at least one non-static method and does not already have a __reactstandin__regenerateByEval method, add a __reactstandin__regenerateByEval method. It seems like it should be fine to instead just add the method unconditionally, which would be simpler, but let me know if that would cause problems. There's already a place where we insert code at the start of every class body (to implement class fields for classes without constructors), so I think this should be reasonable.
  If there is an export default [expression] statement, extract it into a temp variable and add it as another binding. This will probably need special cases for when the imports transform is enabled/disabled, but I think should be possible.

We are looking for the ways without __reactstandin__regenerateByEval command. Unfortunately, it does not working - gaearon/react-hot-loader#1001.

__reactstandin__regenerateByEval was created to "inject" a new code, into the old component. The problem - it updates the code "as it exists", not "as it was created".
If you have used any "factory" method (reselect, HOC, visa versa) to define class property - it will fail.
I dont need __reactstandin__regenerateByEval, I need contructor to be callable. But ES6 classes does not allow it.

Working theory - change React, or RHL, to work "without" proxy, on hot update - replace the class as a whole, and copy some props (state, variables) from old instance to a new one.
Result - no problems with updates, no need of regenerate, and no sense for you to implement.
This is something we have to do, and doing right now.

[alangpierce]

@theKashey Looks like there have been a few more requests for react-hot-loader support, and the react-hot-loader Babel plugin hasn't had many changes, so I think I'll just implement that. I was actually surprised at how well react-hot-loader works without the plugin, so arguably it's not so important for Sucrase to support it, but looks like __reactstandin__regenerateByEval at least is necessary for bound methods to be updated.

Do you know a specific case where the reactHotLoader.register lines are necessary? RHL seems quite good already at figuring out things like child components, and would be good to make sure I'm testing the right cases.

[theKashey]

register calls could solve some false-positives like merging HOCs like react-loadable all together (gaearon/react-hot-loader#1088 (comment)), cos they are actually looks the same.

Webpack loader could do the same job.

__reactstandin__regenerateByEval is very required by the current approach, but it's a dead end.
This proposal should fix most of React-Hot-Loader issues, and it does not require any eval injection magic by design.

TLDR - feel free to do nothing :)

[alangpierce]

I implemented the transform in #376, some details, but generally wasn't so bad. But sounds like hopefully RHL will soon enough not need a Babel transform and I can remove it! Mostly I wanted to make sure Sucrase could support RHL in the meantime, and in case the alternative RHL implementation takes a while or ends up running into trouble.

[theKashey]

implementation takes a while or ends up running into trouble.

So true

[alangpierce]

I just published this in Sucrase 3.9.0. I did a fair amount of testing and set up the Sucrase website to use RHL with this config, but people should feel free to file issues if they run into trouble!