Skip to content

fix: mutable props capture #621

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 2 commits into from
Jun 15, 2022
Merged

fix: mutable props capture #621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
193e2a4
fix: mutable props capture
manucorporat Jun 13, 2022
0cace80
Merge branch 'main' into fix-mutable-props-capture
manucorporat Jun 14, 2022
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
25 changes: 19 additions & 6 deletions packages/qwik/src/core/object/store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import {
getProxyFlags,
getProxyTarget,
isConnected,
isMutable,
mutable,
shouldSerialize,
SubscriberMap,
} from './q-object';
Expand Down Expand Up @@ -239,16 +241,23 @@ export const snapshotState = (containerEl: Element): SnapshotResult => {
};

const getObjId = (obj: any): string | null => {
let suffix = '';
if (isMutable(obj)) {
obj = obj.v;
suffix = '%';
}
if (isObject(obj)) {
const target = getProxyTarget(obj);
if (target) {
suffix += '!';
}
const id = objToId.get(normalizeObj(target ?? obj, doc));
if (id !== undefined) {
const proxySuffix = target ? '!' : '';
return intToStr(id) + proxySuffix;
return intToStr(id) + suffix;
}
if (!target && isNode(obj)) {
if (obj.nodeType === 1) {
return getElementID(obj as Element);
return getElementID(obj as Element) + suffix;
} else {
logError(codeToText(QError_cannotSerializeNode), obj);
return null;
Expand All @@ -257,7 +266,7 @@ export const snapshotState = (containerEl: Element): SnapshotResult => {
} else {
const id = objToId.get(normalizeObj(obj, doc));
if (id !== undefined) {
return intToStr(id);
return intToStr(id) + suffix;
}
}
return null;
Expand Down Expand Up @@ -578,10 +587,14 @@ const getObjectImpl = (
}
const index = strToInt(id);
assertEqual(objs.length > index, true);
const obj = objs[index];
let obj = objs[index];
const needsProxy = id.endsWith('!');
if (needsProxy && containerState) {
return containerState.$proxyMap$.get(obj) ?? getOrCreateProxy(obj, containerState);
id = id.slice(0, -1);
obj = containerState.$proxyMap$.get(obj) ?? getOrCreateProxy(obj, containerState);
}
if (id.endsWith('%')) {
obj = mutable(obj);
}
return obj;
};
Expand Down
13 changes: 11 additions & 2 deletions starters/apps/e2e/src/components/lexical-scope/lexicalScope.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
import { component$, $, useStore } from '@builder.io/qwik';
import { component$, $, useStore, mutable } from '@builder.io/qwik';

export const LexicalScope = component$(() => {
return <LexicalScopeChild message={mutable('mutable message')}></LexicalScopeChild>;
});

interface LexicalScopeProps {
message: string;
}

export const LexicalScopeChild = component$((props: LexicalScopeProps) => {
const state = useStore({
count: 0,
result: '',
Expand All @@ -24,7 +32,7 @@ export const LexicalScope = component$(() => {
const h = false;

const onclick = $(() => {
state.result = JSON.stringify([a, b, c, String(d), String(e), f, g, h]);
state.result = JSON.stringify([a, b, c, String(d), String(e), f, g, h, props.message]);
state.count++;
});

Expand All @@ -39,6 +47,7 @@ export const LexicalScope = component$(() => {
<p>{JSON.stringify(f)}</p>
<p>{JSON.stringify(g)}</p>
<p>{JSON.stringify(h)}</p>
<p>{props.message}</p>
</div>
<button onClickQrl={onclick} id="rerender">
Rerender {state.count}
Expand Down
4 changes: 2 additions & 2 deletions starters/e2e/e2e.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ test.describe('e2e', () => {
});

test('should rerender without changes', async ({ page }) => {
const SNAPSHOT = `<p>1</p><p>"&lt;/script&gt;"</p><p>{"a":{"thing":12},"b":"hola","c":123,"d":false,"e":true,"f":null,"h":[1,"string",false,{"hola":1},["hello"]]}</p><p>undefined</p><p>null</p><p>[1,2,"hola",{}]</p><p>true</p><p>false</p>`;
const RESULT = `[1,"</script>",{"a":{"thing":12},"b":"hola","c":123,"d":false,"e":true,"f":null,"h":[1,"string",false,{"hola":1},["hello"]]},"undefined","null",[1,2,"hola",{}],true,false]`;
const SNAPSHOT = `<p>1</p><p>"&lt;/script&gt;"</p><p>{"a":{"thing":12},"b":"hola","c":123,"d":false,"e":true,"f":null,"h":[1,"string",false,{"hola":1},["hello"]]}</p><p>undefined</p><p>null</p><p>[1,2,"hola",{}]</p><p>true</p><p>false</p><p>mutable message</p>`;
const RESULT = `[1,"</script>",{"a":{"thing":12},"b":"hola","c":123,"d":false,"e":true,"f":null,"h":[1,"string",false,{"hola":1},["hello"]]},"undefined","null",[1,2,"hola",{}],true,false,"mutable message"]`;

const result = await page.locator('#result');
const content = await page.locator('#static');
Expand Down