fix(minify): only preserve some comments and annotations by default

fi3ework · fi3ework · commit 7a46505a9bf2 · 2024-12-05T14:48:35.000+08:00
diff --git a/packages/core/src/config.ts b/packages/core/src/config.ts
@@ -325,7 +325,8 @@ export function composeMinifyConfig(config: LibConfig): RsbuildConfig {
               toplevel: format !== 'mf',
             },
             format: {
-              comments: 'all',
+              comments: 'some',
+              preserve_annotations: true,
             },
           },
         },
diff --git a/packages/core/tests/__snapshots__/config.test.ts.snap b/packages/core/tests/__snapshots__/config.test.ts.snap
@@ -90,7 +90,8 @@ exports[`Should compose create Rsbuild config correctly > Merge Rsbuild config 1
                 "unused": true,
               },
               "format": {
-                "comments": "all",
+                "comments": "some",
+                "preserve_annotations": true,
               },
               "mangle": false,
               "minify": false,
@@ -327,7 +328,8 @@ exports[`Should compose create Rsbuild config correctly > Merge Rsbuild config 1
                 "unused": true,
               },
               "format": {
-                "comments": "all",
+                "comments": "some",
+                "preserve_annotations": true,
               },
               "mangle": false,
               "minify": false,
@@ -557,7 +559,8 @@ exports[`Should compose create Rsbuild config correctly > Merge Rsbuild config 1
                 "unused": true,
               },
               "format": {
-                "comments": "all",
+                "comments": "some",
+                "preserve_annotations": true,
               },
               "mangle": false,
               "minify": false,
diff --git a/packages/core/tests/config.test.ts b/packages/core/tests/config.test.ts
@@ -331,7 +331,8 @@ describe('minify', () => {
               "unused": true,
             },
             "format": {
-              "comments": "all",
+              "comments": "some",
+              "preserve_annotations": true,
             },
             "mangle": false,
             "minify": false,
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/tests/integration/asset/__snapshots__/index.test.ts.snap b/tests/integration/asset/__snapshots__/index.test.ts.snap
@@ -24,7 +24,7 @@ const SvgLogo = (props)=>/*#__PURE__*/ (0, __WEBPACK_EXTERNAL_MODULE_react_jsx_r
             ]
         })
     });
-/* ESM default export */ const logoreact = SvgLogo;
+const logoreact = SvgLogo;
 console.log(logoreact);
 "
 `;
@@ -53,7 +53,7 @@ const SvgLogo = (props)=>/*#__PURE__*/ (0, __WEBPACK_EXTERNAL_MODULE_react_jsx_r
             ]
         })
     });
-/* ESM default export */ const logoreact = SvgLogo;
+const logoreact = SvgLogo;
 console.log(logoreact);
 "
 `;
diff --git a/tests/integration/bundle-false/__snapshots__/index.test.ts.snap b/tests/integration/bundle-false/__snapshots__/index.test.ts.snap
@@ -24,17 +24,15 @@ const SvgLogo = (props)=>/*#__PURE__*/ (0, __WEBPACK_EXTERNAL_MODULE_react_jsx_r
             ]
         })
     });
-/* ESM default export */ const logoreact = SvgLogo;
-/* ESM default export */ const src_rslib_entry_ = logoreact;
+const logoreact = SvgLogo;
+const src_rslib_entry_ = logoreact;
 export { src_rslib_entry_ as default };
 "
 `;
 
 exports[`svgr in bundleless 2`] = `
 ""use strict";
-// The require scope
 var __webpack_require__ = {};
-/************************************************************************/ // webpack/runtime/define_property_getters
 (()=>{
     __webpack_require__.d = function(exports1, definition) {
         for(var key in definition)if (__webpack_require__.o(definition, key) && !__webpack_require__.o(exports1, key)) Object.defineProperty(exports1, key, {
@@ -43,15 +41,12 @@ var __webpack_require__ = {};
         });
     };
 })();
-// webpack/runtime/has_own_property
 (()=>{
     __webpack_require__.o = function(obj, prop) {
         return Object.prototype.hasOwnProperty.call(obj, prop);
     };
 })();
-// webpack/runtime/make_namespace_object
 (()=>{
-    // define __esModule on exports
     __webpack_require__.r = function(exports1) {
         if ('undefined' != typeof Symbol && Symbol.toStringTag) Object.defineProperty(exports1, Symbol.toStringTag, {
             value: 'Module'
@@ -61,12 +56,10 @@ var __webpack_require__ = {};
         });
     };
 })();
-/************************************************************************/ var __webpack_exports__ = {};
-// ESM COMPAT FLAG
+var __webpack_exports__ = {};
 __webpack_require__.r(__webpack_exports__);
-// EXPORTS
 __webpack_require__.d(__webpack_exports__, {
-    default: ()=>/* binding */ src_rslib_entry_
+    default: ()=>src_rslib_entry_
 });
 const jsx_runtime_namespaceObject = require("react/jsx-runtime");
 require("react");
@@ -91,8 +84,8 @@ const SvgLogo = (props)=>/*#__PURE__*/ (0, jsx_runtime_namespaceObject.jsx)("svg
             ]
         })
     });
-/* ESM default export */ const logoreact = SvgLogo;
-/* ESM default export */ const src_rslib_entry_ = logoreact;
+const logoreact = SvgLogo;
+const src_rslib_entry_ = logoreact;
 var __webpack_export_target__ = exports;
 for(var __webpack_i__ in __webpack_exports__)__webpack_export_target__[__webpack_i__] = __webpack_exports__[__webpack_i__];
 if (__webpack_exports__.__esModule) Object.defineProperty(__webpack_export_target__, '__esModule', {
diff --git a/tests/integration/extension-alias/__snapshots__/index.test.ts.snap b/tests/integration/extension-alias/__snapshots__/index.test.ts.snap
@@ -5,8 +5,6 @@ exports[`resolve.extensionAlias should work 1`] = `
 var __webpack_exports__ = {};
 const bar = 'bar';
 const foo = 'foo';
-// Relative import paths need explicit file extensions in ECMAScript imports
-// when '--moduleResolution' is 'node16' or 'nodenext'.
 console.log(foo + bar);
 var __webpack_export_target__ = exports;
 for(var __webpack_i__ in __webpack_exports__)__webpack_export_target__[__webpack_i__] = __webpack_exports__[__webpack_i__];
@@ -19,8 +17,6 @@ if (__webpack_exports__.__esModule) Object.defineProperty(__webpack_export_targe
 exports[`resolve.extensionAlias should work 2`] = `
 "const bar = 'bar';
 const foo = 'foo';
-// Relative import paths need explicit file extensions in ECMAScript imports
-// when '--moduleResolution' is 'node16' or 'nodenext'.
 console.log(foo + bar);
 "
 `;
diff --git a/tests/integration/minify/__fixtures__/src/index.ts b/tests/integration/minify/__fixtures__/src/index.ts
@@ -1,6 +1,12 @@
+/*! Legal Comment */
+import { jsx } from 'react/jsx-runtime';
+
 export const foo = () => {};
 
 const bar = () => {};
 const baz = () => {
   return bar();
 };
+
+// normal comment
+export const Button = () => /*#__PURE__*/ jsx('button', {});
diff --git a/tests/integration/minify/config/disabled/package.json b/tests/integration/minify/config/disabled/package.json
@@ -0,0 +1,6 @@
+{
+  "name": "minify-diabled-config-test",
+  "version": "1.0.0",
+  "private": true,
+  "type": "module"
+}
diff --git a/tests/integration/minify/config/disabled/rslib.config.ts b/tests/integration/minify/config/disabled/rslib.config.ts
@@ -6,10 +6,11 @@ export default defineConfig({
   lib: [generateBundleEsmConfig()],
   output: {
     minify: false,
+    externals: ['react/jsx-runtime'],
   },
   source: {
     entry: {
-      index: path.resolve(__dirname, '../__fixtures__/src/index.ts'),
+      index: path.resolve(__dirname, '../../__fixtures__/src/index.ts'),
     },
   },
 });
diff --git a/tests/integration/minify/config/enabled/package.json b/tests/integration/minify/config/enabled/package.json
@@ -1,5 +1,5 @@
 {
-  "name": "minify-config-test",
+  "name": "minify-true-config-test",
   "version": "1.0.0",
   "private": true,
   "type": "module"
diff --git a/tests/integration/minify/config/enabled/rslib.config.ts b/tests/integration/minify/config/enabled/rslib.config.ts
@@ -0,0 +1,26 @@
+import path from 'node:path';
+import { defineConfig } from '@rslib/core';
+import { generateBundleEsmConfig } from 'test-helper';
+
+export default defineConfig({
+  lib: [generateBundleEsmConfig()],
+  output: {
+    minify: {
+      js: true,
+      jsOptions: {
+        minimizerOptions: {
+          format: {
+            comments: 'some',
+            preserve_annotations: true,
+          },
+        },
+      },
+    },
+    externals: ['react/jsx-runtime'],
+  },
+  source: {
+    entry: {
+      index: path.resolve(__dirname, '../../__fixtures__/src/index.ts'),
+    },
+  },
+});
diff --git a/tests/integration/minify/default/rslib.config.ts b/tests/integration/minify/default/rslib.config.ts
@@ -9,4 +9,7 @@ export default defineConfig({
       index: path.resolve(__dirname, '../__fixtures__/src/index.ts'),
     },
   },
+  output: {
+    externals: ['react/jsx-runtime'],
+  },
 });
diff --git a/tests/integration/minify/index.test.ts b/tests/integration/minify/index.test.ts
@@ -2,59 +2,88 @@ import { join } from 'node:path';
 import { buildAndGetResults } from 'test-helper';
 import { expect, test } from 'vitest';
 
-test('tree shaking is enabled by default, bar and baz should be shaken', async () => {
+test('tree shaking is enabled by default, bar and baz should be shaken, some comments and annotations are preserved', async () => {
   const fixturePath = join(__dirname, 'default');
   const { entries } = await buildAndGetResults({ fixturePath });
   expect(entries.esm).toMatchInlineSnapshot(`
-    "const foo = ()=>{};
-    export { foo };
+    "/*! For license information please see index.js.LICENSE.txt */
+    import * as __WEBPACK_EXTERNAL_MODULE_react_jsx_runtime__ from "react/jsx-runtime";
+    /*! Legal Comment */ const foo = ()=>{};
+    const Button = ()=>/*#__PURE__*/ (0, __WEBPACK_EXTERNAL_MODULE_react_jsx_runtime__.jsx)('button', {});
+    export { Button, foo };
     "
   `);
 });
 
-test('tree shaking is disabled by the user, bar and baz should be kept', async () => {
-  const fixturePath = join(__dirname, 'config');
+test('minify is disabled, nothing will be stripped', async () => {
+  const fixturePath = join(__dirname, 'config/disabled');
   const { entries } = await buildAndGetResults({ fixturePath });
   expect(entries.esm).toMatchInlineSnapshot(`
-    "
-    ;// CONCATENATED MODULE: ../__fixtures__/src/index.ts?__rslib_entry__
+    "import * as __WEBPACK_EXTERNAL_MODULE_react_jsx_runtime__ from "react/jsx-runtime";
+
+    ;// CONCATENATED MODULE: external "react/jsx-runtime"
+
+    ;// CONCATENATED MODULE: ../../__fixtures__/src/index.ts?__rslib_entry__
+    /*! Legal Comment */ 
     const foo = ()=>{};
     const bar = ()=>{};
     const baz = ()=>{
         return bar();
     };
+    // normal comment
+    const Button = ()=>/*#__PURE__*/ (0,__WEBPACK_EXTERNAL_MODULE_react_jsx_runtime__.jsx)('button', {});
 
-    export { foo };
+    export { Button, foo };
     "
   `);
 });
 
-test('minify is enabled by default in mf format, bar and baz should be minified', async () => {
+test('minify is enabled, only preserve some comments and annotations', async () => {
+  const fixturePath = join(__dirname, 'config/enabled');
+  const { entries } = await buildAndGetResults({ fixturePath });
+  expect(entries.esm).toMatchInlineSnapshot(`
+    "/*! For license information please see index.js.LICENSE.txt */
+    import*as t from"react/jsx-runtime";/*! Legal Comment */let o=()=>{},r=()=>/*#__PURE__*/(0,t.jsx)("button",{});export{r as Button,o as foo};"
+  `);
+});
+
+test('minify is enabled by default in mf format, bar and baz should be shaken, some comments and annotations are preserved', async () => {
   const fixturePath = join(__dirname, 'mf/default');
   const { mfExposeEntry } = await buildAndGetResults({ fixturePath });
   // biome-ignore format: snapshot
-  expect(mfExposeEntry).toMatchInlineSnapshot(`""use strict";(globalThis["default_minify"]=globalThis["default_minify"]||[]).push([["249"],{163:function(__unused_webpack_module,__webpack_exports__,__webpack_require__){__webpack_require__.r(__webpack_exports__);__webpack_require__.d(__webpack_exports__,{foo:function(){return foo}});const foo=()=>{}}}]);"`);
+  expect(mfExposeEntry).toMatchInlineSnapshot(`
+    "/*! For license information please see __federation_expose_default_export.js.LICENSE.txt */
+    "use strict";(globalThis["default_minify"]=globalThis["default_minify"]||[]).push([["249"],{163:function(__unused_webpack_module,__webpack_exports__,__webpack_require__){__webpack_require__.r(__webpack_exports__);__webpack_require__.d(__webpack_exports__,{Button:function(){return Button},foo:function(){return foo}});var react_jsx_runtime__WEBPACK_IMPORTED_MODULE_0__=__webpack_require__(37);/*! Legal Comment */const foo=()=>{};const Button=()=>/*#__PURE__*/(0,react_jsx_runtime__WEBPACK_IMPORTED_MODULE_0__.jsx)("button",{})}}]);"
+  `);
 });
 
-test('minify is disabled by the user, bar and baz should not be minified', async () => {
+test('minify is disabled by the user, nothing not be stripped', async () => {
   const fixturePath = join(__dirname, 'mf/config');
   const { mfExposeEntry } = await buildAndGetResults({ fixturePath });
 
-  expect(mfExposeEntry).toMatchInlineSnapshot(`""use strict";
-(globalThis['disable_minify'] = globalThis['disable_minify'] || []).push([["249"], {
-"163": (function (__unused_webpack_module, __webpack_exports__, __webpack_require__) {
-__webpack_require__.r(__webpack_exports__);
-__webpack_require__.d(__webpack_exports__, {
-  foo: function() { return foo; }
-});
-const foo = ()=>{};
-const bar = ()=>{};
-const baz = ()=>{
-    return bar();
-};
+  expect(mfExposeEntry).toMatchInlineSnapshot(`
+    ""use strict";
+    (globalThis['disable_minify'] = globalThis['disable_minify'] || []).push([["249"], {
+    "163": (function (__unused_webpack_module, __webpack_exports__, __webpack_require__) {
+    __webpack_require__.r(__webpack_exports__);
+    __webpack_require__.d(__webpack_exports__, {
+      Button: function() { return Button; },
+      foo: function() { return foo; }
+    });
+    /* ESM import */var react_jsx_runtime__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(37);
+    /* ESM import */var react_jsx_runtime__WEBPACK_IMPORTED_MODULE_0___default = /*#__PURE__*/__webpack_require__.n(react_jsx_runtime__WEBPACK_IMPORTED_MODULE_0__);
+    /*! Legal Comment */ 
+    const foo = ()=>{};
+    const bar = ()=>{};
+    const baz = ()=>{
+        return bar();
+    };
+    // normal comment
+    const Button = ()=>/*#__PURE__*/ (0,react_jsx_runtime__WEBPACK_IMPORTED_MODULE_0__.jsx)('button', {});
 
 
-}),
+    }),
 
-}]);"`);
+    }]);"
+  `);
 });
diff --git a/tests/integration/minify/mf/config/rslib.config.ts b/tests/integration/minify/mf/config/rslib.config.ts
@@ -30,6 +30,7 @@ export default defineConfig({
   output: {
     target: 'web',
     minify: false,
+    externals: ['react/jsx-runtime'],
   },
   source: {
     entry: {
diff --git a/tests/integration/minify/mf/default/rslib.config.ts b/tests/integration/minify/mf/default/rslib.config.ts
@@ -34,5 +34,6 @@ export default defineConfig({
   },
   output: {
     target: 'web',
+    externals: ['react/jsx-runtime'],
   },
 });
diff --git a/tests/integration/resolve/index.test.ts b/tests/integration/resolve/index.test.ts
@@ -8,7 +8,7 @@ test('resolve data url', async () => {
 
   expect(isSuccess).toBeTruthy();
   expect(entries.esm).toMatchInlineSnapshot(`
-    "/* ESM default export */ const javascript_export_default_42 = 42;
+    "const javascript_export_default_42 = 42;
     console.log('x:', javascript_export_default_42);
     "
   `);
diff --git a/tests/integration/shims/index.test.ts b/tests/integration/shims/index.test.ts
@@ -111,10 +111,8 @@ describe('CJS shims', () => {
       const importMetaUrl = import.meta.url;
       const src_rslib_entry_require = (0, __WEBPACK_EXTERNAL_MODULE_node_module__.createRequire)(import.meta.url);
       const requiredModule = src_rslib_entry_require('./ok.cjs');
-      // https://github.com/web-infra-dev/rslib/issues/425
       const src_rslib_entry_filename = (0, __WEBPACK_EXTERNAL_MODULE_url__.fileURLToPath)(import.meta.url);
       console.log(src_rslib_entry_filename);
-      // https://github.com/web-infra-dev/rslib/pull/399
       const src_rslib_entry_module = null;
       export { src_rslib_entry_filename as __filename, importMetaUrl, src_rslib_entry_module as module, requiredModule };
       "

Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ const SvgLogo = (props)=>/#__PURE__/ (0, __WEBPACK_EXTERNAL_MODULE_react_jsx_r`
`24`	`24`	`]`
`25`	`25`	`})`
`26`	`26`	`});`
`27`		`-/* ESM default export */ const logoreact = SvgLogo;`
	`27`	`+const logoreact = SvgLogo;`
`28`	`28`	`console.log(logoreact);`
`29`	`29`	`"`
`30`	`30`	`;
`@@ -53,7 +53,7 @@ const SvgLogo = (props)=>/#__PURE__/ (0, __WEBPACK_EXTERNAL_MODULE_react_jsx_r`
`53`	`53`	`]`
`54`	`54`	`})`
`55`	`55`	`});`
`56`		`-/* ESM default export */ const logoreact = SvgLogo;`
	`56`	`+const logoreact = SvgLogo;`
`57`	`57`	`console.log(logoreact);`
`58`	`58`	`"`
`59`	`59`	`;