Skip to content

Commit

Permalink
fix(modern-module): concatenate entry module regardless bail reasons (#…
Browse files Browse the repository at this point in the history
…8165)

fix(modern-module): concaten entry module regardless bail reasons
  • Loading branch information
fi3ework authored Oct 24, 2024
1 parent 11f20e1 commit 7f11c4f
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 7 deletions.
11 changes: 5 additions & 6 deletions crates/rspack_plugin_library/src/modern_module_library_plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,12 @@ impl ModernModuleLibraryPlugin {
.module_graph_module_by_identifier(id)
.expect("should have module");
let reasons = &mgm.optimization_bailout;
reasons

let is_concatenation_entry_candidate = reasons
.iter()
// We did want force concatenate entry point here.
// TODO: use constant variable to identify the reason.
.filter(|r| !r.contains("Module is an entry point"))
.collect::<Vec<_>>()
.is_empty()
.any(|r| r.contains("Module is an entry point"));

is_concatenation_entry_candidate
})
.collect::<HashSet<_>>();

Expand Down
80 changes: 80 additions & 0 deletions packages/rspack-test-tools/tests/__snapshots__/Config.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,86 @@ const value = foo + (bar_default())
export { value };
`;
exports[`config config/library/modern-module-force-concaten step should pass: harmony export should concat, even with bailout reason 1`] = `
var __webpack_modules__ = ({
"974": (function (module) {
module.exports = 'foo'
}),
});
/************************************************************************/
// The module cache
var __webpack_module_cache__ = {};
// The require function
function __webpack_require__(moduleId) {
// Check if module is in cache
var cachedModule = __webpack_module_cache__[moduleId];
if (cachedModule !== undefined) {
return cachedModule.exports;
}
// Create a new module (and put it into the cache)
var module = (__webpack_module_cache__[moduleId] = {
exports: {}
});
// Execute the module function
__webpack_modules__[moduleId](module, module.exports, __webpack_require__);
// Return the exports of the module
return module.exports;
}
/************************************************************************/
// webpack/runtime/compat_get_default_export
(() => {
// getDefaultExport function for compatibility with non-ESM modules
__webpack_require__.n = function (module) {
var getter = module && module.__esModule ?
function () { return module['default']; } :
function () { return module; };
__webpack_require__.d(getter, { a: getter });
return getter;
};
})();
// webpack/runtime/define_property_getters
(() => {
__webpack_require__.d = function(exports, definition) {
for(var key in definition) {
if(__webpack_require__.o(definition, key) && !__webpack_require__.o(exports, key)) {
Object.defineProperty(exports, key, { enumerable: true, get: definition[key] });
}
}
};
})();
// webpack/runtime/has_own_property
(() => {
__webpack_require__.o = function (obj, prop) {
return Object.prototype.hasOwnProperty.call(obj, prop);
};
})();
/************************************************************************/
// EXTERNAL MODULE: ./g/foo.js
var foo = __webpack_require__("974");
var foo_default = /*#__PURE__*/__webpack_require__.n(foo);
;// CONCATENATED MODULE: ./g/index.js
var __webpack_exports__foo = (foo_default());
export { __webpack_exports__foo as foo };
`;
exports[`config config/library/modern-module-force-concaten step should pass: unambiguous should bail out 1`] = `const c = 'c'`;
exports[`config config/optimization/minimizer-esm-asset exported tests minimizing an asset file of esm type should success 1`] = `console.log(import.meta.url);export const a=1;`;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = 'foo'
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import foo from './foo.js'

export { foo }
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ module.exports = {
"c": "./c.js",
"d": "./d.mjs",
"e": "./e/index.js",
"f": "./f/index.js"
"f": "./f/index.js",
"g": "./g/index.js"
},
externals: {
path: 'node-commonjs path',
Expand Down Expand Up @@ -39,6 +40,7 @@ module.exports = {
expect(assets['d.js']._value).toMatchSnapshot(".mjs should concat");
expect(assets['e.js']._value).toMatchSnapshot(".cjs should bail out when bundling");
expect(assets['f.js']._value).toMatchSnapshot("external module should bail out when bundling");
expect(assets['g.js']._value).toMatchSnapshot("harmony export should concat, even with bailout reason");
});
};
this.hooks.compilation.tap("testcase", handler);
Expand Down

2 comments on commit 7f11c4f

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Benchmark detail: Open

Name Base (2024-10-24 6f99ff3) Current Change
10000_development-mode + exec 2.11 s ± 48 ms 2.11 s ± 40 ms +0.36 %
10000_development-mode_hmr + exec 668 ms ± 22 ms 670 ms ± 15 ms +0.29 %
10000_production-mode + exec 2.71 s ± 48 ms 2.69 s ± 40 ms -0.76 %
arco-pro_development-mode + exec 1.8 s ± 66 ms 1.76 s ± 71 ms -1.71 %
arco-pro_development-mode_hmr + exec 426 ms ± 1.6 ms 427 ms ± 2.4 ms +0.15 %
arco-pro_production-mode + exec 3.19 s ± 73 ms 3.23 s ± 62 ms +1.14 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.24 s ± 69 ms 3.24 s ± 57 ms -0.19 %
threejs_development-mode_10x + exec 1.61 s ± 17 ms 1.61 s ± 12 ms -0.02 %
threejs_development-mode_10x_hmr + exec 761 ms ± 20 ms 762 ms ± 6.9 ms +0.08 %
threejs_production-mode_10x + exec 5.01 s ± 29 ms 5.01 s ± 26 ms +0.10 %

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Ran ecosystem CI: Open

suite result
modernjs ✅ success
_selftest ✅ success
rspress ✅ success
rslib ✅ success
rsbuild ✅ success
examples ✅ success
devserver ✅ success

Please sign in to comment.