Skip to content

Commit

Permalink
fix(optimizer): minification breaks imports and exports (#122)
Browse files Browse the repository at this point in the history 
fixes #119
  • Loading branch information
@manucorporat
manucorporat authored Dec 28, 2021
1 parent f8394c6 commit b4c8b20
Show file tree
Hide file tree
Showing 6 changed files with 169 additions and 70 deletions.
28 changes: 17 additions & 11 deletions src/optimizer/core/src/code_move.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::collector::{GlobalCollect, ImportKind};
use crate::collector::{new_ident_from_id, GlobalCollect, Id, ImportKind};
use crate::parse::{emit_source_code, HookAnalysis, PathData, TransformModule, TransformOutput};

use std::collections::HashMap;
Expand All @@ -15,7 +15,7 @@ pub fn new_module(
path: &PathData,
name: &str,
origin: &str,
local_idents: &[JsWord],
local_idents: &[Id],
global: &GlobalCollect,
) -> Result<(ast::Module, SingleThreadedComments), Error> {
let comments = SingleThreadedComments::default();
Expand All @@ -25,26 +25,27 @@ pub fn new_module(
body: Vec::with_capacity(max_cap),
shebang: None,
};
for ident in local_idents {
if let Some(import) = global.imports.get(ident) {

for id in local_idents {
if let Some(import) = global.imports.get(id) {
let specifier = match import.kind {
ImportKind::Named => ast::ImportSpecifier::Named(ast::ImportNamedSpecifier {
is_type_only: false,
span: DUMMY_SP,
imported: if &import.specifier == ident {
imported: if import.specifier == id.0 {
None
} else {
Some(ast::Ident::new(import.specifier.clone(), DUMMY_SP))
},
local: ast::Ident::new(ident.clone(), DUMMY_SP),
local: new_ident_from_id(id),
}),
ImportKind::Default => ast::ImportSpecifier::Default(ast::ImportDefaultSpecifier {
span: DUMMY_SP,
local: ast::Ident::new(ident.clone(), DUMMY_SP),
local: new_ident_from_id(id),
}),
ImportKind::All => ast::ImportSpecifier::Namespace(ast::ImportStarAsSpecifier {
span: DUMMY_SP,
local: ast::Ident::new(ident.clone(), DUMMY_SP),
local: new_ident_from_id(id),
}),
};
module
Expand All @@ -63,7 +64,12 @@ pub fn new_module(
specifiers: vec![specifier],
},
)));
} else if let Some(export) = global.exports.get(ident) {
} else if let Some(export) = global.exports.get(id) {
let imported = if export == &id.0 {
None
} else {
Some(ast::Ident::new(export.clone(), DUMMY_SP))
};
module
.body
.push(ast::ModuleItem::ModuleDecl(ast::ModuleDecl::Import(
Expand All @@ -80,8 +86,8 @@ pub fn new_module(
specifiers: vec![ast::ImportSpecifier::Named(ast::ImportNamedSpecifier {
is_type_only: false,
span: DUMMY_SP,
imported: None,
local: ast::Ident::new(export.clone(), DUMMY_SP),
imported,
local: new_ident_from_id(id),
})],
},
)));
Expand Down
28 changes: 21 additions & 7 deletions src/optimizer/core/src/collector.rs
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,29 @@
use std::collections::{BTreeMap, HashSet};

use swc_atoms::{js_word, JsWord};
use swc_common::{BytePos, Span, SyntaxContext};
use swc_ecmascript::ast;
use swc_ecmascript::visit::{noop_visit_type, visit_expr, visit_stmt, Visit, VisitWith};

macro_rules! id {
($ident: expr) => {
$ident.sym.clone()
($ident.sym.clone(), $ident.span.ctxt())
};
}

pub type Id = (JsWord, SyntaxContext);

pub fn new_ident_from_id(id: &Id) -> ast::Ident {
ast::Ident::new(
id.0.clone(),
Span {
lo: BytePos(0),
hi: BytePos(0),
ctxt: id.1,
},
)
}

#[derive(PartialEq, Clone, Copy)]
pub enum ImportKind {
Named,
Expand All @@ -24,8 +38,8 @@ pub struct Import {
}

pub struct GlobalCollect {
pub imports: BTreeMap<JsWord, Import>,
pub exports: BTreeMap<JsWord, JsWord>,
pub imports: BTreeMap<Id, Import>,
pub exports: BTreeMap<Id, JsWord>,
in_export_decl: bool,
}

Expand Down Expand Up @@ -177,7 +191,7 @@ enum ExprOrSkip {
#[derive(Debug)]
pub struct HookCollect {
pub local_decl: HashSet<JsWord>,
pub local_idents: HashSet<JsWord>,
pub local_idents: HashSet<Id>,
expr_ctxt: Vec<ExprOrSkip>,
}

Expand All @@ -192,9 +206,9 @@ impl HookCollect {
collect
}

pub fn get_words(self) -> (Vec<JsWord>, Vec<JsWord>) {
pub fn get_words(self) -> (Vec<JsWord>, Vec<Id>) {
let mut local_decl: Vec<JsWord> = self.local_decl.into_iter().collect();
let mut local_idents: Vec<JsWord> = self.local_idents.into_iter().collect();
let mut local_idents: Vec<Id> = self.local_idents.into_iter().collect();
local_idents.sort();
local_decl.sort();
(local_decl, local_idents)
Expand Down Expand Up @@ -422,7 +436,7 @@ impl Visit for HookCollect {

fn visit_ident(&mut self, node: &ast::Ident) {
if let Some(ExprOrSkip::Expr) = self.expr_ctxt.last() {
self.local_idents.insert(node.sym.clone());
self.local_idents.insert(id!(node));
}
}

Expand Down
95 changes: 46 additions & 49 deletions src/optimizer/core/src/parse.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,8 @@ use swc_ecmascript::minifier::option::{
use swc_ecmascript::parser::lexer::Lexer;
use swc_ecmascript::parser::{EsConfig, PResult, Parser, StringInput, Syntax, TsConfig};
use swc_ecmascript::transforms::{
fixer,
hygiene::{self, hygiene_with_config},
optimization::simplify,
pass, react, resolver_with_mark, typescript,
fixer, hygiene::hygiene_with_config, optimization::simplify, pass, react, resolver_with_mark,
typescript,
};
use swc_ecmascript::visit::{FoldWith, VisitMutWith};

Expand Down Expand Up @@ -142,28 +140,13 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a

swc_common::GLOBALS.set(&Globals::new(), || {
swc_common::errors::HANDLER.set(&handler, || {
let mut react_options = react::Options::default();
if is_jsx {
react_options.use_spread = true;
react_options.import_source = "@builder.io/qwik".to_string();
react_options.pragma = "h".to_string();
react_options.pragma_frag = "Fragment".to_string();
};

let collect = global_collect(&main_module);
let global_mark = Mark::fresh(Mark::root());

let mut hooks: Vec<Hook> = vec![];
let mut main_module = main_module;

main_module.visit_mut_with(&mut resolver_with_mark(global_mark));

let mut main_module = {
// Transpile JSX
if transpile && is_type_script {
let mut passes = chain!(
pass::Optional::new(
typescript::strip(global_mark),
transpile && is_type_script && !is_jsx
),
pass::Optional::new(typescript::strip(global_mark), !is_jsx),
pass::Optional::new(
typescript::strip_with_jsx(
Lrc::clone(&source_map),
Expand All @@ -175,31 +158,46 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a
Some(&comments),
global_mark,
),
transpile && is_type_script && is_jsx
),
pass::Optional::new(
react::react(
Lrc::clone(&source_map),
Some(&comments),
react_options,
global_mark
),
transpile && is_jsx
),
HookTransform::new(
config.context,
&path_data,
config.entry_policy,
Some(&comments),
&mut hooks
is_jsx
),
);
main_module.fold_with(&mut passes)
};
main_module = main_module.fold_with(&mut passes)
}

// Resolve with mark
main_module.visit_mut_with(&mut resolver_with_mark(global_mark));

// Transpile JSX
if transpile && is_jsx {
let mut react_options = react::Options::default();
if is_jsx {
react_options.use_spread = true;
react_options.import_source = "@builder.io/qwik".to_string();
react_options.pragma = "h".to_string();
react_options.pragma_frag = "Fragment".to_string();
};
main_module = main_module.fold_with(&mut react::react(
Lrc::clone(&source_map),
Some(&comments),
react_options,
global_mark,
));
}

// Collect import/export metadata
let collect = global_collect(&main_module);
let mut hooks: Vec<Hook> = vec![];

// Run main transform
main_module = main_module.fold_with(&mut HookTransform::new(
config.context,
&path_data,
config.entry_policy,
Some(&comments),
&mut hooks,
));

if config.minify != MinifyMode::None {
let top_level_mark = Mark::fresh(Mark::root());
main_module.visit_mut_with(&mut resolver_with_mark(top_level_mark));
main_module =
main_module.fold_with(&mut simplify::simplifier(Default::default()));

Expand All @@ -221,7 +219,9 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a
wrap: false,
enclose: false,
},
&ExtraOptions { top_level_mark },
&ExtraOptions {
top_level_mark: global_mark,
},
);
}

Expand Down Expand Up @@ -253,7 +253,6 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a
)?;

if config.minify == MinifyMode::Minify {
hook_module = hook_module.fold_with(&mut resolver_with_mark(hook_mark));
hook_module = optimize(
hook_module,
Lrc::clone(&source_map),
Expand All @@ -277,9 +276,7 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a
);

hook_module = hook_module
.fold_with(&mut hygiene_with_config(hygiene::Config {
..Default::default()
}))
.fold_with(&mut hygiene_with_config(Default::default()))
.fold_with(&mut fixer(None));
}

Expand All @@ -298,7 +295,7 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a
entry: h.entry,
canonical_filename: h.canonical_filename,
local_decl: h.local_decl,
local_idents: h.local_idents,
local_idents: h.local_idents.into_iter().map(|id| id.0).collect(),
});
modules.push(TransformModule {
code,
Expand Down
56 changes: 56 additions & 0 deletions src/optimizer/core/src/snapshots/qwik_core__test__issue_118.snap
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
---
source: src/optimizer/core/src/test.rs
expression: output

---
==INPUT==


import {qHook, h} from '@builderio/qwik';
import thing from 'lib';
import * as all from 'lib';
import {s as se} from 'lib';


export const Header = qComponent({
onMount: <div/>,
onRender: qHook(() => <Footer>{thing}{all()}{se()}</Footer>)
});

export const Footer = qComponent();



============================= project/test.js ==

import{qHook as a,h as b}from"@builderio/qwik";export const Header=/*#__PURE__*/ qComponent({onMount:/*#__PURE__*/ b("div",null),onRender:a(()=>import("../entry_hooks"),"Header_onRender")});export const Footer=/*#__PURE__*/ qComponent();
============================= h_test_header_onrender.js ==

import{Footer as a}from"./project/test";import*as b from"lib";import{h as c}from"@builderio/qwik";import{qHook as d}from"@builderio/qwik";import{s as e}from"lib";import f from"lib";export const Header_onRender=/*#__PURE__*/ d(()=>c(a,null,f,b(),e()));
============================= entry_hooks.js (ENTRY POINT)==

export { Header_onRender } from "./h_test_header_onrender";

== HOOKS ==

[
{
"origin": "project/test.tsx",
"name": "Header_onRender",
"entry": "entry_hooks",
"canonicalFilename": "h_test_header_onrender",
"localDecl": [],
"localIdents": [
"Footer",
"all",
"h",
"qHook",
"se",
"thing"
]
}
]

== DIAGNOSTICS ==

[]
26 changes: 26 additions & 0 deletions src/optimizer/core/src/test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,32 @@ export const cache = patternCache[cacheKey] || (patternCache[cacheKey]={});
);
}

#[test]
fn issue_118() {
test_input(
"project/test.tsx",
r#"
import {qHook, h} from '@builderio/qwik';
import thing from 'lib';
import * as all from 'lib';
import {s as se} from 'lib';
export const Header = qComponent({
onMount: <div/>,
onRender: qHook(() => <Footer>{thing}{all()}{se()}</Footer>)
});
export const Footer = qComponent();
"#,
EntryStrategy::Single,
MinifyMode::Minify,
true,
);
}

// fn test_fixture(folder: &str) {
// let res = transform_workdir(&FSConfig {
// project_root: folder.to_string(),
Expand Down
Loading

0 comments on commit b4c8b20

Please sign in to comment.