Skip to content
This repository was archived by the owner on Apr 8, 2020. It is now read-only.

Updating aspnet-webpack-react to work with react-hot-loader v4 #1675

Closed
wants to merge 2 commits into from

Conversation

tjaskula
Copy link
Contributor

@tjaskula tjaskula commented Jun 7, 2018

This should solve the issue #1585

  • Deleting references to react-hot-loader/webpack and react-hot-loader/patch as it was removed in v4 of react-hot-loader. Referencing react-hot-loader/babelin webpack configuration is enough.
  • Bumping peer dependency of webpack to 3 and 4

…er/patch' as it was removed in v4 of react-hot-loader. Bumping peer dependency of webpack to 3 and 4
@dnfclas
Copy link

dnfclas commented Jun 7, 2018

CLA assistant check
All CLA requirements met.

@natemcmaster natemcmaster changed the base branch from dev to release/2.2 June 30, 2018 06:08
@SteveSandersonMS
Copy link
Member

@natemcmaster I'll look at releasing this. Note that since it's purely a change to an NPM package, it doesn't need to be in any ASP.NET Core release milestone. We can ship NPM package changes at any time; they don't go into ASP.NET Core releases.

@tjaskula Since you're dropping support for Webpack 2 and 3, by removing the reactHotLoaderPatch (etc), shouldn't the peerDependencies allow only Webpack 4, rather than allowing Webpack 2/3/4? Also could you please bump up the major version of aspnet-webpack-react itself to 4.0.0, because this is a semver-major breaking change.

@natemcmaster
Copy link
Contributor

Thanks for taking a look @SteveSandersonMS.

Side note: I was playing around with this over the weekend at seemed to me like improvements to react-hot-loader and webpack 4 make aspnet-webpack-react unnecessary. I got hmr working just fine without it. I'm not proposing anything...just pointing out that I couldn't figure out what aspnet-webpack-react was supposed to do.

@SteveSandersonMS
Copy link
Member

@natemcmaster Were you using the SPA templates from ASP.NET Core 2.0 or later? If so, you're not using aspnet-webpack-react at all. It's only used by pre-2.0 templates.

@natemcmaster
Copy link
Contributor

I see, makes sense. I was setting up hmr with ASP.NET Core 2.1 on an old hobby project I was upgrading from ASP.NET 4. I didn't see HMR in the latest SPA templates so wasn't sure what to do.

@tjaskula
Copy link
Contributor Author

tjaskula commented Jul 4, 2018

@SteveSandersonMS Thanks for suggestions. I'll target Webpack 4 only which will be related to the changes.
@natemcmaster You mean you made working HMR and ASP.NET core (on kestrel) without this module? I don't remeber now but it seems that enabling HMR yielded an error thus this PR.

@natemcmaster
Copy link
Contributor

You mean you made working HMR and ASP.NET core (on kestrel) without this module?

@tjaskula yes. It may require updating to the latest versions of webpack and react-hot-loader, but I got it working just fine without aspnet-webpack-react. I wrote up a quick demo and post about this https://natemcmaster.com/blog/2018/07/05/aspnetcore-hmr/

@tjaskula
Copy link
Contributor Author

tjaskula commented Jul 7, 2018

@natemcmaster I checked what was different in my configuration and realized that in ASP.NET Core I had this configuration:

app.UseWebpackDevMiddleware(new WebpackDevMiddlewareOptions
{
     HotModuleReplacement = true,
     ReactHotModuleReplacement = true,
     ProjectPath = System.IO.Path.Combine(env.ContentRootPath, "../")
});

Notice the flag ReactHotModuleReplacement = true. When this is set, the aspnet-webpack-react dependency is loaded with an error described in the issue #1585.

When I comment out ReactHotModuleReplacement the error goes away (I'm also using ASP.NET Core 2.1)

My point is that, if you're using ASP.NET Core 2.1, just go without aspnet-webpack-react and ReactHotModuleReplacement flag. (as @SteveSandersonMS mentioned it above)

For ASP.NET Core < 2.0 this PR might be of value.

I'm making changes to the PR as requested above.

@cvanem
Copy link

cvanem commented Jul 8, 2018

@tjaskula Thanks for clearing this up. Removing aspnet-webpack-react and REactHotMudleReplacement = true resolved my error and HMR still functions as expected.

@tjaskula
Copy link
Contributor Author

@natemcmaster do sourcemaps works for you ? I noticed that for me it doesn't load properly in Chrome

@natemcmaster
Copy link
Contributor

Yeah, I got those working with the inline-source-map devtool and awesome-typescript-loader.

@tjaskula
Copy link
Contributor Author

All is ok, I was using webpack.SourceMapdevtoolPlugin which seems broken. Swtiching to detool: inline-source-map seems fixing the issue.

… 4 as peerDependency. Bumping up dev dependencies too.
@tjaskula
Copy link
Contributor Author

I've pushed the required changes. Please review if this is ok.

@Tsourdox
Copy link

Really need this one. What's the eta for reviewing this PR?

@Eilon
Copy link
Contributor

Eilon commented Sep 25, 2018

@SteveSandersonMS ?

@SteveSandersonMS
Copy link
Member

Thanks for the prompt. This is now merged and published as aspnet-webpack-react@4.0.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants