Skip to content

aspnet-webpack-react not compatible with react-hot-loader v4 #5178

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

Closed
Guymestef opened this issue Mar 21, 2018 · 19 comments
Closed

aspnet-webpack-react not compatible with react-hot-loader v4 #5178

Guymestef opened this issue Mar 21, 2018 · 19 comments
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-spa

Comments

@Guymestef
Copy link

Hello,

react-hot-loader/patch has been removed from react-hot-loader v4: See documentation here

A new package for aspnet-webpack-react (v4.0.0?), removing support of "reactHotLoaderPatch", need to be created I suppose.

To remove?

const reactHotLoaderPatch = 'react-hot-loader/patch';

To remove?

        let entryValueArray = entryConfig[entrypointName] as string[];
        if (entryValueArray.indexOf(reactHotLoaderPatch) < 0) {
            entryValueArray.unshift(reactHotLoaderPatch);
        }

react-hot-loader release note

@Guymestef
Copy link
Author

For users still using react-hot-loader v3, they would be able to use this new version by adding react-hot-loader/patch to their webpack conf manually. No?

@Guymestef
Copy link
Author

By doing some test, it seems that the module react-hot-loader/webpack has been removed too...

const reactHotLoaderWebpackLoader = 'react-hot-loader/webpack';

@ghost
Copy link

ghost commented Mar 22, 2018

I'm getting the error about 'react-hot-loader/webpack' being missing, so rolling back to V3 for now.

@mark-monteiro
Copy link

The safest solution to this might be to check if the react-hot-loader/patch or react-hot-loader/webpack packages exist before injecting them into the webpack config. Then we won't break compatibility with v3. Would something like this work?

function npmModuleIsPresent(moduleName: string) {
    try {
        require.resolve(moduleName);
        return true;
    } catch (ex) {
        return false;
    }
}

@SteveSandersonMS
Copy link
Member

Would something like this work?

It looks like it would avoid the initial exception, but I'm not sure it solves the issue that HMR wouldn't work. Do you have an idea about how that should be changed for v4?

@mark-monteiro
Copy link

In the migration instructions for migrating from v3 to v4, the only mention of changes to webpack config is the removal of react-hot-loader/patch: https://github.com/gaearon/react-hot-loader#migrating-from-v3

There is also a working example setup for HMR with Typescript that can be referenced: https://github.com/gaearon/react-hot-loader/tree/master/examples/typescript

The only notable thing in that sample Webpack config is adding babel-loader before the Typescript loader which I think we are already doing: https://github.com/aspnet/JavaScriptServices/blob/02beb11a5c1a0f5a32e2345b0708a323c166627e/src/Microsoft.AspNetCore.SpaServices/npm/aspnet-webpack/src/WebpackDevMiddleware.ts#L116

@rydergillen-compacSort
Copy link

rydergillen-compacSort commented Mar 28, 2018

@SteveSanderson

It looks like it would avoid the initial exception, but I'm not sure it solves the issue that HMR wouldn't work. Do you have an idea about how that should be changed for v4?

I would like to add that by removing the following lines (along with usage of variables) from HotModuleReplacement.ts I was able to successfully use webpack 4 & HMR inside of the ASPNET CORE React/Redux template site.

var reactHotLoaderWebpackLoader = 'react-hot-loader/webpack';
var reactHotLoaderPatch = 'react-hot-loader/patch';

If thats the case then @mark-monteiro suggestion of conditionally invoking may just work. Can anyone else confirm this resolves their issue?

@reecreatepatrik
Copy link

It resolved it for me too.

@mark-monteiro
Copy link

When I tested the change it also worked for me as well, but I didn't want to comment on that because I ran through it very briefly and didn't test anything very thoroughly.

That being said, is it fair to say that this would be a good change to make regardless of the status of compatibility with v4? It seems like a prudent measure to make sure a package is installed before trying to use it.

If we make this change then we maintain backwards compatibility without totally breaking people's builds when they try to update to v4 as is the case now. If there is any further configuration changes required for v4 compatibility, then at least there will be the option to make those Webpack config changes manually.

@Tsourdox
Copy link

Tsourdox commented Apr 26, 2018

Has the change been implemented yet? If not, where would be the proper place to add/call the "npmModuleIsPresent" function?

@Tsourdox
Copy link

Tsourdox commented Apr 26, 2018

Tried @rydergillen-compacSort solution, worked for me too!

@Tsourdox
Copy link

Any updates on this issue? 🙏

@feafarot
Copy link

feafarot commented May 29, 2018

Since #1562 was resolved. Maybe this issue also could now be fixed?

@tjaskula
Copy link
Contributor

tjaskula commented Jun 6, 2018

any solution doesn't work for me. Would be good to release a new version compatible with react-hot-loader v4

@cvanem
Copy link

cvanem commented Jun 23, 2018

@SteveSandersonMS I hate to bug you, but would it be possible to get this PR merged, assuming there is nothing holding it up? We have been waiting a while and it would be nice to be able to upgrade. The proposed solution works for myself and others as described above. Is there anything holding this up?

@TimeWanderer
Copy link

Any updates on this?

@tjaskula
Copy link
Contributor

tjaskula commented Aug 1, 2018

Waiting for #1675 being merged

@GGAlanSmithee
Copy link

Seems like this is now fixed, and is working for me aspnet/JavaScriptServices#1675 (comment)

@aspnet-hello aspnet-hello transferred this issue from aspnet/JavaScriptServices Dec 17, 2018
@aspnet-hello aspnet-hello added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-spa labels Dec 17, 2018
@mkArtakMSFT
Copy link
Member

Closing as this seems to be resolved now.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-spa
Projects
None yet
Development

No branches or pull requests