Skip to content

fix(optimizer): let the bundler handle entries #6670

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Aug 1, 2024
Merged

fix(optimizer): let the bundler handle entries #6670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b00905a
fix(optimizer): correct side effect cleaner
wmertens Aug 1, 2024
39f71b1
fix(optimizer): let the bundler handle entries
wmertens Jul 11, 2024
e4777bf
refactor(dev): remove foundql from vite-dev-server
wmertens Aug 1, 2024
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
5 changes: 5 additions & 0 deletions .changeset/ninety-weeks-enjoy.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@builder.io/qwik': minor
---

The optimizer plugin will now rely on Rollup to group QRL segments. It will only provide hints on which segments fit well together. The result of this change is that now code splitting happens during the transform phase only, and other Rollup/Vite plugins (such as css-in-js plugins) can transform the code before Qwik transforms it.
55 changes: 32 additions & 23 deletions packages/qwik/src/optimizer/core/src/clean_side_effects.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,54 +1,63 @@
use swc_common::Mark;
use swc_ecmascript::ast;
use std::cell::RefCell;
use std::collections::HashSet;
use std::rc::Rc;

use swc_common::Span;
use swc_common::Spanned;
use swc_ecmascript::ast::Module;
use swc_ecmascript::ast::{Expr, ModuleItem, Stmt};
use swc_ecmascript::visit::VisitMut;

/// This is a simple treeshaker that removes anything with side effects from the module.
/// These are:
/// - `new` expressions
/// - `call` expressions
///
/// but only when they are not assigned and used elsewhere.
/// but only when they are not assigned and used elsewhere, or when they were already present before simplifying.
///
/// - First it marks top-level expressions
/// - Then the code needs to be simplified by you
/// - Then it removes all top-level expressions that are not marked. Those will be expressions that were
/// assigned to unused variables etc.

pub struct Treeshaker {
pub marker: CleanMarker,
pub cleaner: CleanSideEffects,
}

pub struct CleanSideEffects {
pub did_drop: bool,
pub mark: Mark,
pub struct CleanMarker {
set: Rc<RefCell<HashSet<Span>>>,
}

pub struct CleanMarker {
pub mark: Mark,
pub struct CleanSideEffects {
pub did_drop: bool,
set: Rc<RefCell<HashSet<Span>>>,
}

impl Treeshaker {
pub fn new() -> Self {
let mark = Mark::new();
let set = Rc::new(RefCell::new(HashSet::new()));
Self {
marker: CleanMarker { mark },
marker: CleanMarker {
set: Rc::clone(&set),
},
cleaner: CleanSideEffects {
did_drop: false,
mark,
set: Rc::clone(&set),
},
}
}
}

impl VisitMut for CleanMarker {
fn visit_mut_module_item(&mut self, node: &mut ast::ModuleItem) {
if let ast::ModuleItem::Stmt(ast::Stmt::Expr(expr)) = node {
fn visit_mut_module_item(&mut self, node: &mut ModuleItem) {
if let ModuleItem::Stmt(Stmt::Expr(expr)) = node {
match &*expr.expr {
ast::Expr::New(e) => {
e.ctxt.apply_mark(self.mark);
Expr::New(e) => {
self.set.borrow_mut().insert(e.span());
}
ast::Expr::Call(e) => {
e.ctxt.apply_mark(self.mark);
Expr::Call(e) => {
self.set.borrow_mut().insert(e.span());
}
_ => {}
}
Expand All @@ -57,18 +66,18 @@ impl VisitMut for CleanMarker {
}

impl VisitMut for CleanSideEffects {
fn visit_mut_module(&mut self, node: &mut ast::Module) {
fn visit_mut_module(&mut self, node: &mut Module) {
node.body.retain(|item| match item {
ast::ModuleItem::Stmt(ast::Stmt::Expr(expr)) => match &*expr.expr {
ast::Expr::New(e) => {
if e.ctxt.has_mark(self.mark) {
ModuleItem::Stmt(Stmt::Expr(expr)) => match &*expr.expr {
Expr::New(e) => {
if self.set.borrow().contains(&e.span()) {
return true;
}
self.did_drop = true;
false
}
ast::Expr::Call(e) => {
if e.ctxt.has_mark(self.mark) {
Expr::Call(e) => {
if self.set.borrow().contains(&e.span()) {
return true;
}
self.did_drop = true;
Expand Down
156 changes: 3 additions & 153 deletions packages/qwik/src/optimizer/core/src/code_move.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,12 @@
use crate::collector::{new_ident_from_id, GlobalCollect, Id, ImportKind};
use crate::parse::{
emit_source_code, might_need_handle_watch, HookAnalysis, PathData, TransformModule,
TransformOutput,
};
use crate::parse::PathData;
use crate::transform::{add_handle_watch, create_synthetic_named_import};
use crate::words::*;

use std::collections::BTreeMap;
use std::path::Path;

use anyhow::{Context, Error};
use path_slash::PathExt;
use anyhow::Error;
use swc_atoms::JsWord;
use swc_common::comments::{SingleThreadedComments, SingleThreadedCommentsMap};
use swc_common::{sync::Lrc, SourceMap, DUMMY_SP};
use swc_common::DUMMY_SP;
use swc_ecmascript::ast;
use swc_ecmascript::utils::private_ident;

Expand Down Expand Up @@ -158,32 +151,6 @@ pub fn new_module(ctx: NewModuleCtx) -> Result<(ast::Module, SingleThreadedComme
Ok((module, comments))
}

pub fn fix_path<S: AsRef<Path>, D: AsRef<Path>>(
src: S,
dest: D,
ident: &str,
) -> Result<JsWord, Error> {
let src = src.as_ref();
let dest = dest.as_ref();
if ident.starts_with('.') {
let diff = pathdiff::diff_paths(src, dest);

if let Some(diff) = diff {
let normalize = diff.to_slash_lossy();
let relative = relative_path::RelativePath::new(&normalize);
let final_path = relative.join(ident).normalize();
let final_str = final_path.as_str();
return Ok(if final_str.starts_with('.') {
JsWord::from(final_str)
} else {
JsWord::from(format!("./{}", final_str))
});
}
}

Ok(JsWord::from(ident))
}

fn create_named_export(expr: Box<ast::Expr>, name: &str) -> ast::ModuleItem {
ast::ModuleItem::ModuleDecl(ast::ModuleDecl::ExportDecl(ast::ExportDecl {
span: DUMMY_SP,
Expand All @@ -206,123 +173,6 @@ fn create_named_export(expr: Box<ast::Expr>, name: &str) -> ast::ModuleItem {
}))
}

#[test]
fn test_fix_path() {
assert_eq!(
fix_path("src", "", "./state.qwik.mjs").unwrap(),
JsWord::from("./src/state.qwik.mjs")
);

assert_eq!(
fix_path("src/path", "", "./state").unwrap(),
JsWord::from("./src/path/state")
);

assert_eq!(
fix_path("src", "", "../state").unwrap(),
JsWord::from("./state")
);
assert_eq!(
fix_path("a", "a", "./state").unwrap(),
JsWord::from("./state")
);
}

pub fn generate_entries(
mut output: TransformOutput,
core_module: &JsWord,
explicit_extensions: bool,
root_dir: Option<&Path>,
) -> Result<TransformOutput, anyhow::Error> {
let source_map = Lrc::new(SourceMap::default());
let mut entries_map: BTreeMap<&str, Vec<&HookAnalysis>> = BTreeMap::new();
let mut new_modules = Vec::with_capacity(output.modules.len());
{
let hooks: Vec<&HookAnalysis> = output.modules.iter().flat_map(|m| &m.hook).collect();
for hook in hooks {
if let Some(ref e) = hook.entry {
entries_map.entry(e.as_ref()).or_default().push(hook);
}
}

for (entry, hooks) in &entries_map {
let module = new_entry_module(entry, hooks, core_module, explicit_extensions);
let (code, map) =
emit_source_code(Lrc::clone(&source_map), None, &module, root_dir, false)
.context("Emitting source code")?;
new_modules.push(TransformModule {
path: [entry, ".js"].concat(),
code,
map,
is_entry: true,
hook: None,
order: 0,
});
}
}
output.modules.append(&mut new_modules);

Ok(output)
}

fn new_entry_module(
path: &str,
hooks: &[&HookAnalysis],
core_module: &JsWord,
explicit_extensions: bool,
) -> ast::Module {
let mut module = ast::Module {
span: DUMMY_SP,
body: Vec::with_capacity(hooks.len()),
shebang: None,
};
let mut need_handle_watch = false;
for hook in hooks {
// TODO fix the path from the entry to the hook in case of mismatched location
let mut src = fix_path(
hook.path.to_string(),
Path::new(path).parent().unwrap().to_str().unwrap(),
&["./", &hook.canonical_filename].concat(),
)
.unwrap()
.to_string();
if explicit_extensions {
src = src + "." + hook.extension.as_ref();
}
if might_need_handle_watch(&hook.ctx_kind, &hook.ctx_name) {
need_handle_watch = true;
}
module
.body
.push(ast::ModuleItem::ModuleDecl(ast::ModuleDecl::ExportNamed(
ast::NamedExport {
span: DUMMY_SP,
type_only: false,
with: None,
src: Some(Box::new(ast::Str {
span: DUMMY_SP,
value: JsWord::from(src),
raw: None,
})),
specifiers: vec![ast::ExportSpecifier::Named(ast::ExportNamedSpecifier {
is_type_only: false,
span: DUMMY_SP,
orig: ast::ModuleExportName::Ident(ast::Ident::new(
hook.name.clone(),
DUMMY_SP,
Default::default(),
)),
exported: None,
})],
},
)));
}
if need_handle_watch {
add_handle_watch(&mut module.body, core_module);
}
module
}

pub fn transform_function_expr(
expr: ast::Expr,
use_lexical_scope: &Id,
Expand Down
36 changes: 1 addition & 35 deletions packages/qwik/src/optimizer/core/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ use std::path::Path;
use std::str;
use swc_atoms::JsWord;

use crate::code_move::generate_entries;
use crate::entry_strategy::parse_entry_strategy;
pub use crate::entry_strategy::EntryStrategy;
pub use crate::parse::EmitMode;
Expand Down Expand Up @@ -160,23 +159,7 @@ pub fn transform_fs(config: TransformFsOptions) -> Result<TransformOutput, Error
.reduce(|| Ok(TransformOutput::new()), |x, y| Ok(x?.append(&mut y?)))?;

final_output.modules.sort_unstable_by_key(|key| key.order);
if !matches!(
config.entry_strategy,
EntryStrategy::Hook | EntryStrategy::Inline | EntryStrategy::Hoist
) {
final_output = generate_entries(
final_output,
&core_module,
config.explicit_extensions,
root_dir,
)?;
}
// final_output = generate_entries(
// final_output,
// &core_module,
// config.explicit_extensions,
// root_dir,
// )?;

Ok(final_output)
}

Expand Down Expand Up @@ -231,23 +214,6 @@ pub fn transform_modules(config: TransformModulesOptions) -> Result<TransformOut

let mut final_output = final_output?;
final_output.modules.sort_unstable_by_key(|key| key.order);
if !matches!(
config.entry_strategy,
EntryStrategy::Hook | EntryStrategy::Inline | EntryStrategy::Hoist
) {
final_output = generate_entries(
final_output,
&core_module,
config.explicit_extensions,
root_dir,
)?;
}
// final_output = generate_entries(
// final_output,
// &core_module,
// config.explicit_extensions,
// root_dir,
// )?;

Ok(final_output)
}
4 changes: 1 addition & 3 deletions packages/qwik/src/optimizer/core/src/parse.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a
]
.concat();
let need_handle_watch =
might_need_handle_watch(&h.data.ctx_kind, &h.data.ctx_name) && is_entry;
might_need_handle_watch(&h.data.ctx_kind, &h.data.ctx_name);

let (mut hook_module, comments) = new_module(NewModuleCtx {
expr: h.expr,
Expand Down Expand Up @@ -676,7 +676,6 @@ fn handle_error(
pub struct PathData {
pub abs_path: PathBuf,
pub rel_path: PathBuf,
pub base_dir: PathBuf,
pub abs_dir: PathBuf,
pub rel_dir: PathBuf,
pub file_stem: String,
Expand Down Expand Up @@ -706,7 +705,6 @@ pub fn parse_path(src: &str, base_dir: &Path) -> Result<PathData, Error> {

Ok(PathData {
abs_path,
base_dir: base_dir.to_path_buf(),
rel_path: path.into(),
abs_dir,
rel_dir,
Expand Down
Loading
Loading