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

feat: keep comments for development and production without minify #848

Merged
merged 1 commit into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions crates/mako/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion crates/mako/src/chunk_pot/str_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
11 changes: 8 additions & 3 deletions crates/mako/src/chunk_pot/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down
2 changes: 1 addition & 1 deletion crates/mako/src/plugins/bundless_compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
20 changes: 11 additions & 9 deletions crates/mako/src/transform.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
));
}
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion crates/mako/src/transformers/transform_react.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down
9 changes: 2 additions & 7 deletions e2e/fixtures/assets.svg.svgr/expect.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"
);
7 changes: 7 additions & 0 deletions e2e/fixtures/javascript.keep-comments/expect.js
Original file line number Diff line number Diff line change
@@ -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");
5 changes: 5 additions & 0 deletions e2e/fixtures/javascript.keep-comments/mako.config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"entry": {
"index": "src/index.js"
}
}
2 changes: 2 additions & 0 deletions e2e/fixtures/javascript.keep-comments/src/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
/* foo */
console.log(1);
8 changes: 2 additions & 6 deletions e2e/fixtures/node-polyfill/expect.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"
);
4 changes: 2 additions & 2 deletions e2e/fixtures/optimize-package-imports/expect.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
3 changes: 3 additions & 0 deletions examples/dead-simple/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import { foo } from './foo';
import './foo/foo';
/**
* abcd
*/
console.log(foo);