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

perf: optimize props destructuring #2545

Merged
merged 7 commits into from
Jan 3, 2023
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
1 change: 1 addition & 0 deletions .npmrc
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ engine-strict=true
strict-peer-dependencies=false
shell-emulator=true
auto-install-peers=false
ignore-workspace-root-check=true
20 changes: 10 additions & 10 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,31 +59,31 @@
"@napi-rs/triples": "1.1.0",
"@node-rs/helper": "1.3.3",
"@octokit/action": "3.18.1",
"@playwright/test": "1.29.0",
"@playwright/test": "1.29.1",
"@types/brotli": "1.3.1",
"@types/cross-spawn": "6.0.2",
"@types/eslint": "8.4.10",
"@types/express": "4.17.15",
"@types/mri": "1.1.1",
"@types/node": "^18.11.16",
"@types/node": "^18.11.18",
"@types/node-fetch": "2.6.2",
"@types/path-browserify": "1.0.0",
"@types/prettier": "2.7.1",
"@types/prettier": "2.7.2",
"@types/prompts": "2.4.2",
"@types/semver": "7.3.13",
"@types/which-pm-runs": "1.0.0",
"@typescript-eslint/eslint-plugin": "5.46.1",
"@typescript-eslint/parser": "5.46.1",
"@typescript-eslint/utils": "5.46.1",
"@typescript-eslint/eslint-plugin": "5.48.0",
"@typescript-eslint/parser": "5.48.0",
"@typescript-eslint/utils": "5.48.0",
"all-contributors-cli": "6.24.0",
"brotli": "1.3.3",
"commitizen": "4.2.6",
"concurrently": "7.6.0",
"create-qwik": "workspace:*",
"cross-spawn": "7.0.3",
"cz-conventional-changelog": "3.3.0",
"esbuild": "0.16.8",
"eslint": "8.30.0",
"esbuild": "0.16.12",
"eslint": "8.31.0",
"eslint-plugin-no-only-tests": "3.1.0",
"execa": "6.1.0",
"express": "4.18.2",
Expand All @@ -96,15 +96,15 @@
"prettier": "2.8.1",
"pretty-quick": "^3.1.3",
"prompts": "2.4.2",
"rollup": "3.7.4",
"rollup": "3.9.1",
"semver": "7.3.8",
"snoop": "^1.0.4",
"terser": "5.16.1",
"tsm": "2.2.2",
"typescript": "4.9.4",
"undici": "5.14.0",
"uvu": "0.5.6",
"vite": "4.0.1",
"vite": "4.0.3",
"vite-tsconfig-paths": "4.0.3",
"watchlist": "0.3.1",
"which-pm-runs": "1.1.0"
Expand Down
2 changes: 1 addition & 1 deletion packages/docs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
"typescript": "4.9.4",
"undici": "5.14.0",
"uvu": "0.5.6",
"vite": "4.0.1",
"vite": "4.0.3",
"wrangler": "^2.6.2"
},
"author": "Builder.io Team",
Expand Down
20 changes: 10 additions & 10 deletions packages/eslint-plugin-qwik/qwik.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ export default component$(() => {
return <div></div>
});`,
errors: [
'Identifier ("useMethod") can not be captured inside the scope (component$) because it\'s declared at the root of the module and it is not exported. Add export. Check out https://qwik.builder.io/docs/advanced/optimizer for more details.',
'Identifier ("useMethod") can not be captured inside the scope (component$) because it\'s declared at the root of the module and it is not exported. Add export. Check out https://qwik.builder.io/docs/advanced/dollar/ for more details.',
],
},
{
Expand All @@ -318,7 +318,7 @@ export default component$(() => {
return <div></div>;
});`,
errors: [
'Identifier ("useMethod") can not be captured inside the scope (useTask$) because it is a function, which is not serializable. Check out https://qwik.builder.io/docs/advanced/optimizer for more details.',
'Identifier ("useMethod") can not be captured inside the scope (useTask$) because it is a function, which is not serializable. Check out https://qwik.builder.io/docs/advanced/dollar/ for more details.',
],
},
{
Expand All @@ -334,7 +334,7 @@ export default component$(() => {
});`,

errors: [
'Identifier ("useMethod") can not be captured inside the scope (useTask$) because it is a function, which is not serializable. Check out https://qwik.builder.io/docs/advanced/optimizer for more details.',
'Identifier ("useMethod") can not be captured inside the scope (useTask$) because it is a function, which is not serializable. Check out https://qwik.builder.io/docs/advanced/dollar/ for more details.',
],
},
{
Expand All @@ -348,7 +348,7 @@ export default component$(() => {
});`,

errors: [
'Identifier ("Stuff") can not be captured inside the scope (useTask$) because it is a class constructor, which is not serializable. Check out https://qwik.builder.io/docs/advanced/optimizer for more details.',
'Identifier ("Stuff") can not be captured inside the scope (useTask$) because it is a class constructor, which is not serializable. Check out https://qwik.builder.io/docs/advanced/dollar/ for more details.',
],
},
{
Expand All @@ -363,7 +363,7 @@ export default component$(() => {
});`,

errors: [
'Identifier ("stuff") can not be captured inside the scope (useTask$) because it is an instance of the "Stuff" class, which is not serializable. Use a simple object literal instead. Check out https://qwik.builder.io/docs/advanced/optimizer for more details.',
'Identifier ("stuff") can not be captured inside the scope (useTask$) because it is an instance of the "Stuff" class, which is not serializable. Use a simple object literal instead. Check out https://qwik.builder.io/docs/advanced/dollar/ for more details.',
],
},
{
Expand All @@ -378,7 +378,7 @@ export default component$(() => {
});`,

errors: [
'Identifier ("a") can not be captured inside the scope (useTask$) because it is Symbol, which is not serializable. Check out https://qwik.builder.io/docs/advanced/optimizer for more details.',
'Identifier ("a") can not be captured inside the scope (useTask$) because it is Symbol, which is not serializable. Check out https://qwik.builder.io/docs/advanced/dollar/ for more details.',
],
},
{
Expand All @@ -400,7 +400,7 @@ export default component$(() => {
});`,

errors: [
'Identifier ("a") can not be captured inside the scope (useTask$) because it is a function, which is not serializable. Check out https://qwik.builder.io/docs/advanced/optimizer for more details.',
'Identifier ("a") can not be captured inside the scope (useTask$) because it is a function, which is not serializable. Check out https://qwik.builder.io/docs/advanced/dollar/ for more details.',
],
},
{
Expand All @@ -417,7 +417,7 @@ export default component$(() => {
return <div></div>
});`,
errors: [
'Identifier ("state") can not be captured inside the scope (useTask$) because "state.value" is a function, which is not serializable. Check out https://qwik.builder.io/docs/advanced/optimizer for more details.',
'Identifier ("state") can not be captured inside the scope (useTask$) because "state.value" is a function, which is not serializable. Check out https://qwik.builder.io/docs/advanced/dollar/ for more details.',
],
},
{
Expand Down Expand Up @@ -448,7 +448,7 @@ export default component$(() => {
);
});`,
errors: [
'The value of the identifier ("click") can not be changed once it is captured the scope (onClick$). Check out https://qwik.builder.io/docs/advanced/optimizer for more details.',
'The value of the identifier ("click") can not be changed once it is captured the scope (onClick$). Check out https://qwik.builder.io/docs/advanced/dollar/ for more details.',
],
},
{
Expand All @@ -465,7 +465,7 @@ export default component$(() => {
);
});`,
errors: [
'Identifier ("props") can not be captured inside the scope (onClick$) because "props.nonserializableTuple" is an instance of the "Function" class, which is not serializable. Check out https://qwik.builder.io/docs/advanced/optimizer for more details.',
'Identifier ("props") can not be captured inside the scope (onClick$) because "props.nonserializableTuple" is an instance of the "Function" class, which is not serializable. Check out https://qwik.builder.io/docs/advanced/dollar/ for more details.',
],
},
],
Expand Down
4 changes: 2 additions & 2 deletions packages/eslint-plugin-qwik/src/validLexicalScope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ export const validLexicalScope = createRule({

messages: {
referencesOutside:
'Identifier ("{{varName}}") can not be captured inside the scope ({{dollarName}}) because {{reason}}. Check out https://qwik.builder.io/docs/advanced/optimizer for more details.',
'Identifier ("{{varName}}") can not be captured inside the scope ({{dollarName}}) because {{reason}}. Check out https://qwik.builder.io/docs/advanced/dollar/ for more details.',
unvalidJsxDollar:
'JSX attributes that end with $ can only take an inlined arrow function of a QRL identifier. Make sure the value is created using $()',
mutableIdentifier:
'The value of the identifier ("{{varName}}") can not be changed once it is captured the scope ({{dollarName}}). Check out https://qwik.builder.io/docs/advanced/optimizer for more details.',
'The value of the identifier ("{{varName}}") can not be changed once it is captured the scope ({{dollarName}}). Check out https://qwik.builder.io/docs/advanced/dollar/ for more details.',
},
},
create(context) {
Expand Down
8 changes: 4 additions & 4 deletions packages/qwik-city/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,13 @@
"@netlify/edge-functions": "^2.0.0",
"@types/marked": "4.0.8",
"@types/mdast": "^3.0.10",
"@types/node": "^18.11.16",
"@types/node": "^18.11.18",
"@types/refractor": "3.0.2",
"estree-util-value-to-estree": "2.1.0",
"github-slugger": "2.0.0",
"hast-util-heading-rank": "2.1.0",
"hast-util-to-string": "2.0.0",
"marked": "4.2.4",
"marked": "4.2.5",
"mdast-util-mdx": "^2.0.0",
"refractor": "4.8.0",
"rehype-autolink-headings": "6.1.1",
Expand All @@ -99,8 +99,8 @@
"unified": "10.1.2",
"unist-util-visit": "4.1.1",
"uvu": "0.5.6",
"vite": "4.0.1",
"yaml": "2.1.3"
"vite": "4.0.3",
"yaml": "2.2.1"
},
"peerDependencies": {
"@builder.io/qwik": ">=0.16.0"
Expand Down
4 changes: 2 additions & 2 deletions packages/qwik-react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@
"devDependencies": {
"@builder.io/qwik": "workspace:*",
"@types/react": "18.0.26",
"@types/react-dom": "18.0.9",
"@types/react-dom": "18.0.10",
"react": "18.2.0",
"react-dom": "18.2.0",
"typescript": "4.9.4",
"vite": "4.0.1"
"vite": "4.0.3"
},
"peerDependencies": {
"@builder.io/qwik": ">=0.1.11",
Expand Down
3 changes: 3 additions & 0 deletions packages/qwik/src/core/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,9 @@ export interface ResourceResolved<T> {
// @public (undocumented)
export type ResourceReturn<T> = ResourcePending<T> | ResourceResolved<T> | ResourceRejected<T>;

// @internal (undocumented)
export const _restProps: (props: Record<string, any>, omit: string[]) => Record<string, any>;

// @alpha
export const setPlatform: (plt: CorePlatform) => CorePlatform;

Expand Down
1 change: 1 addition & 0 deletions packages/qwik/src/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ export type { ValueOrPromise } from './util/types';
export type { Signal } from './state/signal';
export type { NoSerialize } from './state/common';
export { _wrapSignal } from './state/signal';
export { _restProps } from './state/store';
export { noSerialize, mutable } from './state/common';
export { _IMMUTABLE } from './state/constants';

Expand Down
21 changes: 13 additions & 8 deletions packages/qwik/src/core/state/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,19 @@ export const setObjectFlags = (obj: object, flags: number) => {

export type TargetType = Record<string | symbol, any>;

/**
* @internal
*/
export const _restProps = (props: Record<string, any>, omit: string[]) => {
const rest: Record<string, any> = {};
for (const key in props) {
if (!omit.includes(key)) {
rest[key] = props[key];
}
}
return rest;
};

class ReadWriteProxyHandler implements ProxyHandler<TargetType> {
constructor(
private $containerState$: ContainerState,
Expand Down Expand Up @@ -174,14 +187,6 @@ class ReadWriteProxyHandler implements ProxyHandler<TargetType> {
}

ownKeys(target: TargetType): ArrayLike<string | symbol> {
let subscriber: SubscriberHost | SubscriberEffect | null | undefined = null;
const invokeCtx = tryGetInvokeContext();
if (invokeCtx) {
subscriber = invokeCtx.$subscriber$;
}
if (subscriber) {
this.$manager$.$addSub$([0, subscriber, undefined]);
}
if (isArray(target)) {
return Reflect.ownKeys(target);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/qwik/src/optimizer/core/src/code_move.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ fn transform_fn(node: ast::FnExpr, use_lexical_scope: &Id, scoped_idents: &[Id])
}
}

const fn create_return_stmt(expr: Box<ast::Expr>) -> ast::Stmt {
pub const fn create_return_stmt(expr: Box<ast::Expr>) -> ast::Stmt {
ast::Stmt::Return(ast::ReturnStmt {
arg: Some(expr),
span: DUMMY_SP,
Expand Down
16 changes: 13 additions & 3 deletions packages/qwik/src/optimizer/core/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ pub struct Import {
}

pub struct GlobalCollect {
pub synthetic: Vec<(Id, Import)>,
pub imports: HashMap<Id, Import>,
pub exports: HashMap<Id, Option<JsWord>>,
pub root: HashMap<Id, Span>,
Expand All @@ -52,6 +53,7 @@ pub struct GlobalCollect {

pub fn global_collect(module: &ast::Module) -> GlobalCollect {
let mut collect = GlobalCollect {
synthetic: vec![],
imports: HashMap::with_capacity(16),
exports: HashMap::with_capacity(16),

Expand Down Expand Up @@ -96,6 +98,9 @@ impl GlobalCollect {
}

pub fn add_import(&mut self, local: Id, import: Import) {
if import.synthetic {
self.synthetic.push((local.clone(), import.clone()));
}
self.rev_imports.insert(
(import.specifier.clone(), import.source.clone()),
local.clone(),
Expand Down Expand Up @@ -370,25 +375,29 @@ impl Visit for IdentCollector {
}
}

pub fn collect_from_pat(pat: &ast::Pat, identifiers: &mut Vec<(Id, Span)>) {
pub fn collect_from_pat(pat: &ast::Pat, identifiers: &mut Vec<(Id, Span)>) -> bool {
match pat {
ast::Pat::Ident(ident) => {
identifiers.push((id!(ident.id), ident.id.span));
true
}
ast::Pat::Array(array) => {
for el in array.elems.iter().flatten() {
collect_from_pat(el, identifiers);
}
false
}
ast::Pat::Rest(rest) => {
if let ast::Pat::Ident(ident) = rest.arg.as_ref() {
identifiers.push((id!(ident.id), ident.id.span));
}
false
}
ast::Pat::Assign(expr) => {
if let ast::Pat::Ident(ident) = expr.left.as_ref() {
identifiers.push((id!(ident.id), ident.id.span));
}
false
}
ast::Pat::Object(obj) => {
for prop in &obj.props {
Expand All @@ -406,7 +415,8 @@ pub fn collect_from_pat(pat: &ast::Pat, identifiers: &mut Vec<(Id, Span)>) {
}
}
}
false
}
_ => {}
};
_ => false,
}
}
1 change: 1 addition & 0 deletions packages/qwik/src/optimizer/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ mod filter_exports;
mod is_immutable;
mod package_json;
mod parse;
mod props_destructuring;
mod transform;
mod utils;
mod words;
Expand Down
6 changes: 5 additions & 1 deletion packages/qwik/src/optimizer/core/src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::collector::global_collect;
use crate::const_replace::ConstReplacerVisitor;
use crate::entry_strategy::EntryPolicy;
use crate::filter_exports::StripExportsVisitor;
use crate::props_destructuring::transform_props_destructuring;
use crate::transform::{HookKind, QwikTransform, QwikTransformOptions};
use crate::utils::{Diagnostic, DiagnosticCategory, DiagnosticScope, SourceLocation};
use path_slash::PathExt;
Expand Down Expand Up @@ -283,8 +284,11 @@ pub fn transform_code(config: TransformCodeOptions) -> Result<TransformOutput, a
));

// Collect import/export metadata
let collect = global_collect(&main_module);
let mut collect = global_collect(&main_module);

transform_props_destructuring(&mut main_module, &mut collect);

// Replace const values
if let Some(is_server) = config.is_server {
let mut const_replacer = ConstReplacerVisitor::new(is_server, &collect);
main_module.visit_mut_with(&mut const_replacer);
Expand Down
Loading