feat: keep comments for development and production without minify

umijs · Jan 8, 2024 · 1f73e88 · 1f73e88
1 parent 5d76e22
commit 1f73e88
Show file tree

Hide file tree

Showing 13 changed files with 54 additions and 35 deletions.
diff --git a/crates/mako/src/ast.rs b/crates/mako/src/ast.rs
@@ -181,19 +181,22 @@ pub fn js_ast_to_code(
     let mut buf = vec![];
     let mut source_map_buf = Vec::new();
     let cm = &context.meta.script.cm;
-    let comments = context.meta.script.output_comments.read().unwrap();
+    let comments = context.meta.script.origin_comments.read().unwrap();
     let swc_comments = comments.get_swc_comments();
     {
+        let with_minify = context.config.minify && matches!(context.config.mode, Mode::Production);
         let mut emitter = Emitter {
             cfg: JsCodegenConfig::default()
-                .with_minify(
-                    context.config.minify && matches!(context.config.mode, Mode::Production),
-                )
+                .with_minify(with_minify)
                 .with_target(context.config.output.es_version)
                 .with_ascii_only(context.config.output.ascii_only)
                 .with_omit_last_semi(true),
             cm: cm.clone(),
-            comments: Some(swc_comments),
+            comments: if with_minify {
+                None
+            } else {
+                Some(swc_comments)
+            },
             wr: Box::new(JsWriter::new(
                 cm.clone(),
                 "\n",

diff --git a/crates/mako/src/chunk_pot/str_impl.rs b/crates/mako/src/chunk_pot/str_impl.rs
@@ -153,7 +153,7 @@ fn to_module_line(
     let mut buf = vec![];
     let mut source_map_buf = Vec::new();
     let cm = context.meta.script.cm.clone();
-    let comments = context.meta.script.output_comments.read().unwrap();
+    let comments = context.meta.script.origin_comments.read().unwrap();
     let swc_comments = comments.get_swc_comments();
 
     let mut emitter = Emitter {

diff --git a/crates/mako/src/chunk_pot/util.rs b/crates/mako/src/chunk_pot/util.rs
@@ -33,17 +33,22 @@ pub(crate) fn render_module_js(
     let mut buf = vec![];
     let mut source_map_buf = Vec::new();
     let cm = context.meta.script.cm.clone();
-    let comments = context.meta.script.output_comments.read().unwrap();
+    let with_minify = context.config.minify && matches!(context.config.mode, Mode::Production);
+    let comments = context.meta.script.origin_comments.read().unwrap();
     let swc_comments = comments.get_swc_comments();
 
     let mut emitter = Emitter {
         cfg: JsCodegenConfig::default()
-            .with_minify(context.config.minify && matches!(context.config.mode, Mode::Production))
+            .with_minify(with_minify)
             .with_target(context.config.output.es_version)
             .with_ascii_only(false)
             .with_omit_last_semi(true),
         cm: cm.clone(),
-        comments: Some(swc_comments),
+        comments: if with_minify {
+            None
+        } else {
+            Some(swc_comments)
+        },
         wr: Box::new(JsWriter::new(cm, "\n", &mut buf, Some(&mut source_map_buf))),
     };
     emitter.emit_module(ast)?;

diff --git a/crates/mako/src/plugins/bundless_compiler.rs b/crates/mako/src/plugins/bundless_compiler.rs
@@ -223,7 +223,7 @@ pub fn transform_js_generate(
                             //         && matches!(context.config.mode, Mode::Production)
                             //     {
                             //         let comments =
-                            //             context.meta.script.output_comments.read().unwrap();
+                            //             context.meta.script.origin_comments.read().unwrap();
                             //         let mut unused_statement_sweep =
                             //             UnusedStatementSweep::new(id, &comments);
                             //         ast.ast.visit_mut_with(&mut unused_statement_sweep);

diff --git a/crates/mako/src/transform.rs b/crates/mako/src/transform.rs
@@ -1,7 +1,6 @@
 use std::sync::Arc;
 
 use mako_core::anyhow::Result;
-use mako_core::swc_common::comments::NoopComments;
 use mako_core::swc_common::errors::HANDLER;
 use mako_core::swc_common::pass::Optional;
 use mako_core::swc_common::sync::Lrc;
@@ -77,14 +76,17 @@ fn transform_js(
             HELPERS.set(&Helpers::new(true), || {
                 HANDLER.set(handler, || {
                     ast.visit_mut_with(&mut resolver(unresolved_mark, top_level_mark, false));
+
+                    let origin_comments = context.meta.script.origin_comments.read().unwrap();
+
                     // strip should be ts only
                     // since when use this in js, it will remove all unused imports
                     // which is not expected as what webpack does
                     if is_ts {
                         ast.visit_mut_with(&mut strip_with_jsx(
                             cm.clone(),
                             Default::default(),
-                            NoopComments,
+                            origin_comments.get_swc_comments(),
                             top_level_mark,
                         ));
                     }
@@ -137,7 +139,7 @@ fn transform_js(
                     // TODO: polyfill
                     let preset_env = swc_preset_env::preset_env(
                         unresolved_mark,
-                        Some(NoopComments),
+                        Some(origin_comments.get_swc_comments()),
                         swc_preset_env::Config {
                             mode: Some(swc_preset_env::Mode::Entry),
                             targets: Some(targets::swc_preset_env_targets_from_map(
@@ -283,8 +285,8 @@ Object.defineProperty(exports, "__esModule", {
     value: true
 });
 var _jsxdevruntime = __mako_require__("react/jsx-dev-runtime");
-const App = ()=>(0, _jsxdevruntime.jsxDEV)(_jsxdevruntime.Fragment, {
-        children: (0, _jsxdevruntime.jsxDEV)("h1", {
+const App = ()=>/*#__PURE__*/ (0, _jsxdevruntime.jsxDEV)(_jsxdevruntime.Fragment, {
+        children: /*#__PURE__*/ (0, _jsxdevruntime.jsxDEV)("h1", {
             children: "Hello World"
         }, void 0, false, {
             fileName: "test.tsx",
@@ -340,7 +342,7 @@ Object.defineProperty(exports, "__esModule", {
     value: true
 });
 var _interop_require_default = __mako_require__("@swc/helpers/_/_interop_require_default");
-var _foo = _interop_require_default._(__mako_require__("foo"));
+var _foo = /*#__PURE__*/ _interop_require_default._(__mako_require__("foo"));
 _foo.default;
 const b = 1;
 b;
@@ -391,7 +393,7 @@ Object.defineProperty(exports, "__esModule", {
     value: true
 });
 var _interop_require_wildcard = __mako_require__("@swc/helpers/_/_interop_require_wildcard");
-var _foo = _interop_require_wildcard._(__mako_require__("./foo"));
+var _foo = /*#__PURE__*/ _interop_require_wildcard._(__mako_require__("./foo"));
 _foo.bar;
 
 //# sourceMappingURL=index.js.map
@@ -486,7 +488,7 @@ Object.defineProperty(exports, "__esModule", {
     value: true
 });
 var _interop_require_default = __mako_require__("@swc/helpers/_/_interop_require_default");
-var _react = _interop_require_default._(__mako_require__("react"));
+var _react = /*#__PURE__*/ _interop_require_default._(__mako_require__("react"));
 _react.default;
 
 //# sourceMappingURL=index.js.map
@@ -702,7 +704,7 @@ Object.defineProperty(exports, "__esModule", {
     value: true
 });
 var _interop_require_default = __mako_require__("@swc/helpers/_/_interop_require_default");
-var _nodefs = _interop_require_default._(require("node:fs"));
+var _nodefs = /*#__PURE__*/ _interop_require_default._(require("node:fs"));
 const fs2 = require('fs');
 const fs3 = require('fs/promises');
 console.log(_nodefs.default, fs2, fs3);

diff --git a/crates/mako/src/transformers/transform_react.rs b/crates/mako/src/transformers/transform_react.rs
@@ -64,11 +64,12 @@ pub fn mako_react(
         noop()
     };
 
+    let origin_comments = context.meta.script.origin_comments.read().unwrap();
     let visit = chain!(
         emotion,
         react(
             cm,
-            Some(NoopComments),
+            Some(origin_comments.get_swc_comments().clone()),
             Options {
                 import_source: Some(import_source.to_string()),
                 pragma: Some(pragma.into()),

diff --git a/e2e/fixtures/assets.svg.svgr/expect.js b/e2e/fixtures/assets.svg.svgr/expect.js
@@ -18,12 +18,7 @@ assert.match(
   ),
   "person.svg's content is not correct"
 );
-assert.match(
-  content,
-  moduleReg(
-    "src/assets/person.svg",
-    'const SvgComponent = (props)=>(0, _jsxdevruntime.jsxDEV)("svg", {',
-    true
-  ),
+assert(
+  content.includes(`const SvgComponent = (props)=>/*#__PURE__*/ (0, _jsxdevruntime.jsxDEV)("svg", {`),
   "person.svg's content is not correct"
 );
diff --git a/e2e/fixtures/javascript.keep-comments/expect.js b/e2e/fixtures/javascript.keep-comments/expect.js
@@ -0,0 +1,7 @@
+const assert = require("assert");
+const { parseBuildResult } = require("../../../scripts/test-utils");
+
+const { files } = parseBuildResult(__dirname);
+const content = files["index.js"];
+
+assert(content.includes(`/* foo */`), "comments should be kept");
diff --git a/e2e/fixtures/javascript.keep-comments/mako.config.json b/e2e/fixtures/javascript.keep-comments/mako.config.json
@@ -0,0 +1,5 @@
+{
+  "entry": {
+    "index": "src/index.js"
+  }
+}
diff --git a/e2e/fixtures/javascript.keep-comments/src/index.js b/e2e/fixtures/javascript.keep-comments/src/index.js
@@ -0,0 +1,2 @@
+/* foo */
+console.log(1);
diff --git a/e2e/fixtures/node-polyfill/expect.js b/e2e/fixtures/node-polyfill/expect.js
@@ -14,11 +14,7 @@ assert.match(
   moduleReg("fs/promise", "module.exports = '';"),
   "should have empty module: fs/promise"
 );
-assert.match(
-  content,
-  moduleReg(
-    "src/index.tsx",
-    'var _path = _interop_require_default._\\(__mako_require__\\("[\\s\\S]*node_modules/node-libs-browser-okam/polyfill/path.js"\\)\\);'
-  ),
+assert(
+  content.includes(`var _path = /*#__PURE__*/ _interop_require_default._(__mako_require__("../../../node_modules/.pnpm/[email protected]/node_modules/node-libs-browser-okam/polyfill/path.js"));`),
   "should have polyfill module: path"
 );
diff --git a/e2e/fixtures/optimize-package-imports/expect.js b/e2e/fixtures/optimize-package-imports/expect.js
@@ -6,13 +6,13 @@ const names = Object.keys(files).join(",");
 const content = files["index.js"];
 
 assert(!content.includes("a-side-effects-false/index.js"), "should not include a-side-effects-false/index.js (barrel file)");
-assert(content.includes(`var _a1export = _interop_require_wildcard._(__mako_require__("node_modules/a-side-effects-false/a1-export.js"));`), "should include a1-export.js");
+assert(content.includes(`var _a1export = /*#__PURE__*/ _interop_require_wildcard._(__mako_require__("node_modules/a-side-effects-false/a1-export.js"));`), "should include a1-export.js");
 assert(content.includes(`var _a2import = __mako_require__("node_modules/a-side-effects-false/a2-import.js");`), "should include a1-import.js");
 assert(content.includes(`var _a3exportfrom = __mako_require__("node_modules/a-side-effects-false/a3-export-from.js");`), "should include a3-export-from.js");
 assert(content.includes(`var _a4 = __mako_require__("node_modules/a-side-effects-false/a4.js");`), "should include a4.js");
 assert(content.includes(`_a4.a41`), "should include _a4.a41");
 assert(content.includes(`var _a5 = __mako_require__("node_modules/a-side-effects-false/a5.js");`), "should include a5.js");
-assert(content.includes(`var _a61 = _interop_require_default._(__mako_require__("node_modules/a-side-effects-false/a61.js"));`), "should include a61.js");
+assert(content.includes(`var _a61 = /*#__PURE__*/ _interop_require_default._(__mako_require__("node_modules/a-side-effects-false/a61.js"));`), "should include a61.js");
 
 assert(content.includes(`var _bsideeffectstrue = __mako_require__("node_modules/b-side-effects-true/index.js");`), "should include a-side-effects-true/index.js (barrel file) but sideEffects: true");
 

diff --git a/examples/dead-simple/index.ts b/examples/dead-simple/index.ts
@@ -1,3 +1,6 @@
 import { foo } from './foo';
 import './foo/foo';
+/**
+ * abcd
+ */
 console.log(foo);