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

Latest 0.24 compiled npm package cause issue in app built with webpack #589

Closed
3cp opened this issue Sep 14, 2020 · 8 comments · Fixed by #633
Closed

Latest 0.24 compiled npm package cause issue in app built with webpack #589

3cp opened this issue Sep 14, 2020 · 8 comments · Fixed by #633

Comments

@3cp
Copy link

3cp commented Sep 14, 2020

Related to #579

Demo repo to reproduce the issue: https://github.com/3cp/ncc-bug

The readme of the demo repo

Project "t" is a lib built with ncc 0.24, dist file is committed. It
has one external lib "lodash", that's where it triggered the error.

Project "t2" is a webpack built with webpack, linking to "t".

To reproduce the error:

cd t2/
npm install
npm start

Navigate browser to http://127.0.0.1:8080 to see the error in console.
To display better error, can add webpack.config.js

module.exports = {
  entry: './src/index.js',
  output: {
    filename: 'main.js',
    path: path.resolve(__dirname, 'dist'),
  },
  mode: 'development',
  devtool: 'cheap-module-eval-source-map';
};

The cause: somehow the __webpack_require__ in t/dist/index.js conflicts
with global var in webpack app t2.
I conform this error went away by manually rename __webpack_require__ in
t/dist/index.js to something else.

In my understanding of js syntax, I don't know how this is possible, because
the __webpack_require__ in t/dist/index.js was inside a function scope.

Unless webpack is doing some code transforming without proper scope analysis
(I guess it assumes var __webpack_require__ is the global var).

Update: see below for the real cause.

@3cp
Copy link
Author

3cp commented Sep 14, 2020

@guybedford I got the cause:

In t/dist/index.js, it leaves external dep lodash like this:

/***/ 804:
/***/ ((module) => {

"use strict";
module.exports = require("lodash");

/***/ })

But in t2, the webpack app rewrote that require call into __webpack__require__.

module.exports = __webpack_require__(/*! lodash */ \"./node_modules/t/node_modules/lodash/lodash.js\");

The problem with 0.24.0 generated code is that the above line is still in the same function scope of local function __webpack_require__ definition, resulting wrong call when loading lodash.

In 0.23.0, the code structure is different, the above line has no visibility of local function __webpack_require__.

Ref: from webpack t2 app dist/main.js.

    (/*!**************************************!*\
  !*** ./node_modules/t/dist/index.js ***!
  \**************************************/
    /*! no static exports found */
    /***/
    function(module, exports, __webpack_require__) {

        eval("/* WEBPACK VAR INJECTION */(function(__dirname) {module.exports =\n/******/ (() => { // webpackBootstrap\n/******/ \tvar __webpack_modules__ = ({\n\n/***/ 932:\n/***/ ((__unused_webpack_module, exports, __webpack_require__) => {\n\nexports._ = __webpack_require__(804);\n\n\n/***/ }),\n\n/***/ 804:\n/***/ ((module) => {\n\n\"use strict\";\nmodule.exports = __webpack_require__(/*! lodash */ \"./node_modules/t/node_modules/lodash/lodash.js\");\n\n/***/ })\n\n/******/ \t});\n/************************************************************************/\n/******/ \t// The module cache\n/******/ \tvar __webpack_module_cache__ = {};\n/******/ \t\n/******/ \t// The require function\n/******/ \tfunction __webpack_require__(moduleId) {\n/******/ \t\t// Check if module is in cache\n/******/ \t\tif(__webpack_module_cache__[moduleId]) {\n/******/ \t\t\treturn __webpack_module_cache__[moduleId].exports;\n/******/ \t\t}\n/******/ \t\t// Create a new module (and put it into the cache)\n/******/ \t\tvar module = __webpack_module_cache__[moduleId] = {\n/******/ \t\t\t// no module.id needed\n/******/ \t\t\t// no module.loaded needed\n/******/ \t\t\texports: {}\n/******/ \t\t};\n/******/ \t\n/******/ \t\t// Execute the module function\n/******/ \t\tvar threw = true;\n/******/ \t\ttry {\n/******/ \t\t\t__webpack_modules__[moduleId](module, module.exports, __webpack_require__);\n/******/ \t\t\tthrew = false;\n/******/ \t\t} finally {\n/******/ \t\t\tif(threw) delete __webpack_module_cache__[moduleId];\n/******/ \t\t}\n/******/ \t\n/******/ \t\t// Return the exports of the module\n/******/ \t\treturn module.exports;\n/******/ \t}\n/******/ \t\n/************************************************************************/\n/******/ \t/* webpack/runtime/compat */\n/******/ \t\n/******/ \t__webpack_require__.ab = __dirname + \"/\";/************************************************************************/\n/******/ \t// module exports must be returned from runtime so entry inlining is disabled\n/******/ \t// startup\n/******/ \t// Load entry module and return exports\n/******/ \treturn __webpack_require__(932);\n/******/ })()\n;\n/* WEBPACK VAR INJECTION */}.call(this, \"/\"))\n\n//# sourceURL=webpack:///./node_modules/t/dist/index.js?");

    }),
    /***/

@3cp
Copy link
Author

3cp commented Sep 14, 2020

Ref: 0.24.0 generated t/dist/index.js. Note the require("lodash") line can see local function __webpack_require__.

module.exports =
/******/ (() => { // webpackBootstrap
/******/ 	var __webpack_modules__ = ({

/***/ 932:
/***/ ((__unused_webpack_module, exports, __webpack_require__) => {

exports._ = __webpack_require__(804);


/***/ }),

/***/ 804:
/***/ ((module) => {

"use strict";
module.exports = require("lodash");

/***/ })

/******/ 	});
/************************************************************************/
/******/ 	// The module cache
/******/ 	var __webpack_module_cache__ = {};
/******/ 	
/******/ 	// The require function
/******/ 	function __webpack_require__(moduleId) {
/******/ 		// Check if module is in cache
/******/ 		if(__webpack_module_cache__[moduleId]) {
/******/ 			return __webpack_module_cache__[moduleId].exports;
/******/ 		}
/******/ 		// Create a new module (and put it into the cache)
/******/ 		var module = __webpack_module_cache__[moduleId] = {
/******/ 			// no module.id needed
/******/ 			// no module.loaded needed
/******/ 			exports: {}
/******/ 		};
/******/ 	
/******/ 		// Execute the module function
/******/ 		var threw = true;
/******/ 		try {
/******/ 			__webpack_modules__[moduleId](module, module.exports, __webpack_require__);
/******/ 			threw = false;
/******/ 		} finally {
/******/ 			if(threw) delete __webpack_module_cache__[moduleId];
/******/ 		}
/******/ 	
/******/ 		// Return the exports of the module
/******/ 		return module.exports;
/******/ 	}
/******/ 	
/************************************************************************/
/******/ 	/* webpack/runtime/compat */
/******/ 	
/******/ 	__webpack_require__.ab = __dirname + "/";/************************************************************************/
/******/ 	// module exports must be returned from runtime so entry inlining is disabled
/******/ 	// startup
/******/ 	// Load entry module and return exports
/******/ 	return __webpack_require__(932);
/******/ })()
;

@3cp
Copy link
Author

3cp commented Sep 14, 2020

Ref: 0.23.0 generated t/dist/index.js. Note the require("lodash") line can not see local function __webpack_require__.

module.exports =
/******/ (function(modules, runtime) { // webpackBootstrap
/******/ 	"use strict";
/******/ 	// The module cache
/******/ 	var installedModules = {};
/******/
/******/ 	// The require function
/******/ 	function __webpack_require__(moduleId) {
/******/
/******/ 		// Check if module is in cache
/******/ 		if(installedModules[moduleId]) {
/******/ 			return installedModules[moduleId].exports;
/******/ 		}
/******/ 		// Create a new module (and put it into the cache)
/******/ 		var module = installedModules[moduleId] = {
/******/ 			i: moduleId,
/******/ 			l: false,
/******/ 			exports: {}
/******/ 		};
/******/
/******/ 		// Execute the module function
/******/ 		var threw = true;
/******/ 		try {
/******/ 			modules[moduleId].call(module.exports, module, module.exports, __webpack_require__);
/******/ 			threw = false;
/******/ 		} finally {
/******/ 			if(threw) delete installedModules[moduleId];
/******/ 		}
/******/
/******/ 		// Flag the module as loaded
/******/ 		module.l = true;
/******/
/******/ 		// Return the exports of the module
/******/ 		return module.exports;
/******/ 	}
/******/
/******/
/******/ 	__webpack_require__.ab = __dirname + "/";
/******/
/******/ 	// the startup function
/******/ 	function startup() {
/******/ 		// Load entry module and return exports
/******/ 		return __webpack_require__(932);
/******/ 	};
/******/
/******/ 	// run startup
/******/ 	return startup();
/******/ })
/************************************************************************/
/******/ ({

/***/ 804:
/***/ (function(module) {

module.exports = require("lodash");

/***/ }),

/***/ 932:
/***/ (function(__unusedmodule, exports, __webpack_require__) {

exports._ = __webpack_require__(804);


/***/ })

/******/ });

@ataylorme

This comment has been minimized.

@ataylorme

This comment has been minimized.

@FinnWoelm
Copy link

This issue also affects Next.js as of v10.0.2 (which uses vercel/ncc v0.25). The Next.js dependency jsonwebtoken is precompiled with vercel/ncc. The entire page and its dependencies (including jsonwebtoken) are then bundled with webpack, which causes the require statements inside jsonwebtoken to reference the wrong __webpack_require__ function and leads to a fatal error.

See vercel/next.js#20575 for more details.

@FinnWoelm
Copy link

Hi @3cp, not sure if this helps, but everything worked correctly in the repo you shared when I upgraded webpack to v5 and webpack-cli to v4 in t2.

@guybedford
Copy link
Contributor

It appears this is a known bug in Webpack in webpack/webpack#1054.

We should support this in ncc though, so can implement an upstream fix specifically for ncc.

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 a pull request may close this issue.

4 participants