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

feat: hmr, compare old locals to invalidate if needed #692

Closed
wants to merge 3 commits into from

Conversation

goloveychuk
Copy link

@goloveychuk goloveychuk commented Feb 2, 2021

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

This allows self accept css module if locals didn't changed. If changed - propagate.
Inspired by https://github.com/webpack-contrib/style-loader/blob/master/src/index.js#L231
Now new runtime is

// extracted by mini-css-extract-plugin
    if(true) {
      // 1612290403401
      var cssReload = __webpack_require__(/*! ../node_modules/mini-css-extract-plugin/dist/hmr/hotModuleReplacement.js */ "../node_modules/mini-css-extract-plugin/dist/hmr/hotModuleReplacement.js")(module.id, {"publicPath":"","esModule":false,"locals":false});
      var _locals = undefined;
      var isEqualLocals = __webpack_require__(/*! ../node_modules/mini-css-extract-plugin/dist/hmr/isEqualLocals.js */ "../node_modules/mini-css-extract-plugin/dist/hmr/isEqualLocals.js");
      module.hot.dispose(function(data) {
        cssReload();
        data.oldLocals = _locals;
      });
      
      if (!_locals || module.hot.invalidate) {
        if (module.hot.invalidate &&
            module.hot.data &&
            module.hot.data.oldLocals &&
            !isEqualLocals(module.hot.data.oldLocals, _locals)) {
            module.hot.invalidate();
        } else {
          module.hot.accept();
        }
      }
    }
  //# sourceURL=[module]
//# sourceMappingURL=data:applicati

Breaking Changes

Additional Info

@goloveychuk
Copy link
Author

@alexander-akait could you please review? Should I fix tests first (look like all of them are code snapshots)

src/loader.js Outdated
@@ -30,7 +38,15 @@ function hotLoader(content, context) {
...context.options,
locals: !!context.locals,
})});
module.hot.dispose(cssReload);
var _locals = ${JSON.stringify(context.locals)};
var isEqualLocals = require(${loaderUtils.stringifyRequest(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When esModule is true it should be import,

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but previously it has

var cssReload = require(${loaderUtils.stringifyRequest(
        context.context,
        path.join(__dirname, 'hmr/hotModuleReplacement.js')

This should be import too?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

const request = loaderUtils.stringifyRequest(context.context, modulePath);

if (context.esModule) {
return `import ${name} from ${request};`;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexander-akait not sure if webpack module interop will work here, mb need to change to * as $name

@goloveychuk
Copy link
Author

printed output,
image

@goloveychuk
Copy link
Author

@alexander-akait could you please review?

@alexander-akait
Copy link
Member

@goloveychuk Sorry for delay, apparently I missed a notification somewhere 😞 , do not hesitate to ping me again, do you want to finish this?

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 this pull request may close these issues.

None yet

2 participants