From 1f73e888d2509ea78857fe2f8a32e9f8f158fc54 Mon Sep 17 00:00:00 2001 From: sorrycc Date: Mon, 8 Jan 2024 22:29:21 +0800 Subject: [PATCH] feat: keep comments for development and production without minify --- crates/mako/src/ast.rs | 13 +++++++----- crates/mako/src/chunk_pot/str_impl.rs | 2 +- crates/mako/src/chunk_pot/util.rs | 11 +++++++--- crates/mako/src/plugins/bundless_compiler.rs | 2 +- crates/mako/src/transform.rs | 20 ++++++++++--------- .../mako/src/transformers/transform_react.rs | 3 ++- e2e/fixtures/assets.svg.svgr/expect.js | 9 ++------- .../javascript.keep-comments/expect.js | 7 +++++++ .../javascript.keep-comments/mako.config.json | 5 +++++ .../javascript.keep-comments/src/index.js | 2 ++ e2e/fixtures/node-polyfill/expect.js | 8 ++------ .../optimize-package-imports/expect.js | 4 ++-- examples/dead-simple/index.ts | 3 +++ 13 files changed, 54 insertions(+), 35 deletions(-) create mode 100644 e2e/fixtures/javascript.keep-comments/expect.js create mode 100644 e2e/fixtures/javascript.keep-comments/mako.config.json create mode 100644 e2e/fixtures/javascript.keep-comments/src/index.js diff --git a/crates/mako/src/ast.rs b/crates/mako/src/ast.rs index 642600ac4..e437f1421 100644 --- 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 index 4e442569a..c8253dbdb 100644 --- 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 index d3a3427ba..005a6a944 100644 --- 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 index a86e96bf7..7ff00f58d 100644 --- 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 index 85a7fea4d..e536c6981 100644 --- 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,6 +76,9 @@ 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 @@ -84,7 +86,7 @@ fn transform_js( 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 index 0bc5cf13d..c53278fc8 100644 --- 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 index 6ab340c4c..f16e80276 100644 --- 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 new file mode 100644 index 000000000..16d36184c --- /dev/null +++ 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 new file mode 100644 index 000000000..cced59194 --- /dev/null +++ b/e2e/fixtures/javascript.keep-comments/mako.config.json @@ -0,0 +1,5 @@ +{ + "entry": { + "index": "src/index.js" + } +} \ No newline at end of file diff --git a/e2e/fixtures/javascript.keep-comments/src/index.js b/e2e/fixtures/javascript.keep-comments/src/index.js new file mode 100644 index 000000000..97e6072e7 --- /dev/null +++ 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 index 91f5a44e6..e5affd3ff 100644 --- 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/node-libs-browser-okam@2.2.4/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 index 30fff825b..1918f511a 100644 --- 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 index 966aa2bf7..e53c31d05 100644 --- 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);