repack icon indicating copy to clipboard operation
repack copied to clipboard

Strict ESM mode supported?

Open DaveWelling opened this issue 2 years ago • 11 comments

If I set my package.json to have "type": "module" for symlinked dependency it causes the runtime code to fail when trying to load interopRequireDefault and react-refresh because require is not defined. It makes sense to me that require would not be defined in these circumstances, but I cannot figure out how to tell webpack to insert hot module reloading, etc. without using require.

I saw that module.hot is not really allowed with strict ESM mode and that import.meta.webpackHot must be used instead (https://webpack.js.org/api/hot-module-replacement/), but I don't have the faintest idea how to do that. It looks like module.hot is specified all over the place in WebpackHMRClients.ts.

I can get rid of the hot module reloading problems by just disabling HMR, but I don't even know where to start with interopRequireDefault or if there are other things I haven't encountered yet that will be inserted in the code using require.

Unfortunately, if I just turn off strict ESM, then mocha, ava, tape, etc. get really confused as to how to deal with the ESM. I'd have to either use babel just to run my tests (which are completely parseable and very fast in node) or use the standard things/esm module. We have thousands of tests, so parsing them all with babel would be a real perf hit on our builds. The standardthings/esm module is not really well supported at this point and things like optional chaining just won't work.

DaveWelling avatar Jul 17 '21 21:07 DaveWelling

I'm not so sure myself. I would start without HMR for simplicity and make it work without it first. My worry is that the strict ESM might not be supported because I terrible usage of ESM and CJS inside React Native itself - mixing ESM with CJS. That might cause some problems.

What's the error with interopRequireDefault exactly?

zamotany avatar Jul 20 '21 12:07 zamotany

Hopefully the react-native code itself won't come into play. It seems like the "type": "module" setting only applies to the code contained in the module defined by the package.json. The setting doesn't make demands on dependencies.

Failing re-pack/nativepack code:

/*!***************************************!*\
  !*** ../lib_ui-services/src/index.js ***!
  \***************************************/
/***/ (function (__unused_webpack___webpack_module__, __webpack_exports__, __webpack_require__) {
    'use strict';
    __webpack_require__.r(__webpack_exports__);
    /* provided dependency */ var __react_refresh_utils__ = __webpack_require__(
        /*! ./node_modules/@pmmmwh/react-refresh-webpack-plugin/lib/runtime/RefreshUtils.js */ './node_modules/@pmmmwh/react-refresh-webpack-plugin/lib/runtime/RefreshUtils.js'
    );
    __webpack_require__.$Refresh$.runtime = require('C:/code/strategic/backbone/silos/silo_ui/backbone20/silo_ui-native/node_modules/react-refresh/runtime.js');
    __webpack_require__.$Refresh$.setup(module.id);

    var _interopRequireDefault = require('@babel/runtime/helpers/interopRequireDefault');
    Object.defineProperty(exports, '__esModule', { value: true });
    Object.defineProperty(exports, 'eventSink', {
        enumerable: true,
        get: function get() {
            return _eventSink.default;
        },
    });
    Object.defineProperty(exports, 'localStorage', {
        enumerable: true,
        get: function get() {
            return _localStorage.default;
        },
    });
    /// ...MORE OBJECT.DEFINES HERE
    exports.metadata = exports.database = exports.http = exports.authentication = void 0;
    var _extends2 = _interopRequireDefault(require('@babel/runtime/helpers/extends'));
    var _index3 = _interopRequireWildcard(require('./database/index'));
    var _eventSink = _interopRequireDefault(require('./eventSink'));
    var _localStorage = _interopRequireDefault(require('./localStorage'));
    /// ...MORE var DECLARATIONS HERE

    /// ... LATER ON IN THE CODE.  I think these exports will probably fail too.
    exports.http = http;
    var database = { get: _index3.default, initialize: _index3.InitializeForUser, isDbReady: _index3.isDbReady };
    exports.database = database;    
});

Here is the same code as generated by my browser webpack build:

/*!***************************************!*\
  !*** ../lib_ui-services/src/index.js ***!
  \***************************************/
/***/ ((__unused_webpack___webpack_module__, __webpack_exports__, __webpack_require__) => {

    "use strict";
    __webpack_require__.r(__webpack_exports__);
    /* harmony export */ __webpack_require__.d(__webpack_exports__, {
    /* harmony export */   "authentication": () => (/* binding */ authentication),
    /* harmony export */   "http": () => (/* binding */ http),
    /* harmony export */   "database": () => (/* binding */ database),
    /* harmony export */   "eventSink": () => (/* reexport safe */ _eventSink__WEBPACK_IMPORTED_MODULE_3__.default),
    // ... MORE EXPORTS HERE
    /* harmony export */ });
    /* harmony import */ var _authentication_index__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! ./authentication/index */ "../lib_ui-services/src/authentication/index.js");
    /* harmony import */ var _http_index__WEBPACK_IMPORTED_MODULE_1__ = __webpack_require__(/*! ./http/index */ "../lib_ui-services/src/http/index.js");
    /* harmony import */ var _database_index__WEBPACK_IMPORTED_MODULE_2__ = __webpack_require__(/*! ./database/index */ "../lib_ui-services/src/database/index.js");
    /* harmony import */ var _eventSink__WEBPACK_IMPORTED_MODULE_3__ = __webpack_require__(/*! ./eventSink */ "../lib_ui-services/src/eventSink.js");
    // ... MORE IMPORTS HERE
            
    const authentication = _authentication_index__WEBPACK_IMPORTED_MODULE_0__;
    const http = { configure: _http_index__WEBPACK_IMPORTED_MODULE_1__.configure, get: _http_index__WEBPACK_IMPORTED_MODULE_1__.get, post: _http_index__WEBPACK_IMPORTED_MODULE_1__.post, remove: _http_index__WEBPACK_IMPORTED_MODULE_1__.remove, patch: _http_index__WEBPACK_IMPORTED_MODULE_1__.patch };
    const database = { get: _database_index__WEBPACK_IMPORTED_MODULE_2__.default, initialize: _database_index__WEBPACK_IMPORTED_MODULE_2__.InitializeForUser, isDbReady: _database_index__WEBPACK_IMPORTED_MODULE_2__.isDbReady };
    /***/ })

DaveWelling avatar Jul 20 '21 17:07 DaveWelling

Have you disabled CommonJS transform in Babel config? Assuming you're using babel-loader?

zamotany avatar Jul 20 '21 18:07 zamotany

Unfortunately, I don't know the correct way to do that. I've been looking through the metro-react-native-babel-preset and I found an option for disableImportExportTransform which seems to avoid adding the "@babel/plugin-transform-modules-commonjs". I'll give that a try.

DaveWelling avatar Jul 20 '21 18:07 DaveWelling

It's looking pretty encouraging so far. If I disable hmr, I am able to get our app to load with the disableImportExportTransform option. For you or anybody that happens to search and find this, here is the relevant webpack.config.js:


module: {
        rules: [
            // Use the default preset with no options for react-native code, etc.
            {
                test: /\.[jt]sx?$/,
                include: [
                    /node_modules(.*[/\\])+react/,
                    /node_modules(.*[/\\])+@react-native/,
                    /node_modules(.*[/\\])+@react-navigation/,
                    /node_modules(.*[/\\])+@react-native-community/,
                    /node_modules(.*[/\\])+@expo/,
                    /node_modules(.*[/\\])+pretty-format/,
                    /node_modules(.*[/\\])+metro/,
                    /node_modules(.*[/\\])+abort-controller/,
                    /node_modules(.*[/\\])+@callstack[/\\]nativepack/
                ],
                use: {
                    loader: 'babel-loader',
                    options: {
                        presets: ['module:metro-react-native-babel-preset']
                    }
                }
            },
            //  Add the  { disableImportExportTransform: true } option for your app specific code
            {
                test: /\.[jt]sx?$/,
                exclude: /node_modules/,
                use: {
                    loader: 'babel-loader',
                    options: {
                        presets: [['module:metro-react-native-babel-preset', { disableImportExportTransform: true }]],
                        /** Add React Refresh transform only when HMR is enabled. */
                        plugins: hmr ? ['module:react-refresh/babel'] : undefined
                    }
                }
            }
      ]
}

DaveWelling avatar Jul 20 '21 19:07 DaveWelling

This is working pretty well for me if drop HMR. I noticed that you put the enhancement label on. I won't close it if you are planning to enhance the HMR capabilities using this work item.

DaveWelling avatar Aug 09 '21 17:08 DaveWelling

Yeah, I want to at least have an official documentation for it and look into seeing if I can get HMR to work with it.

zamotany avatar Aug 10 '21 08:08 zamotany

Based on initial testing it looks like upcoming version 3.x of Re.Pack supports projects with strict ESM mode.

zamotany avatar Jul 11 '22 12:07 zamotany

This is great news. We finally just made it to v2.5.2 so it might take us a little while to get on 3.x. I took a look at https://github.com/callstack/repack/blob/v3/packages/repack/src/modules/WebpackHMRClient.ts and it seems like you are still using module.hot instead of import.meta.webpackHot. I don't think module.hot is supposed to work with ESM modules. Did you figure out a way around that?

DaveWelling avatar Jul 19 '22 20:07 DaveWelling

The ESM as far as I understand in Webpack is scoped, so because WebpackHMRClient.ts is not ESM it can use module.hot. Packages and code that are ESM, have to use import.meta.

The original problem was that @pmmmwh/react-refresh-webpack-plugin was adding CJS style code into ESM code, so you would end up with require/module.hot for ESM-only modules. After some checking, it now adds properly either CJS or ESM style code into modules.

zamotany avatar Jul 20 '22 17:07 zamotany

I know in v1 my initial bundles worked fine, but I had to turn off HMR. I assumed it was because module.hot was causing webpack to use require inside the update bundles. It sounds like I was confusing scopes though and you've established that module.hot vs import.meta.webpackHot is only relevant to the build code - not the code being built. Thanks for the clarification and for continuing to keep this problem in mind.

DaveWelling avatar Jul 20 '22 18:07 DaveWelling

It looks like the problems was solved, closing this due to inactivity. If you need more help, feel free to reopen or create a new issue.

RafikiTiki avatar Dec 28 '22 15:12 RafikiTiki