Skip to content
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
4 changes: 3 additions & 1 deletion docs/content/20-await.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,9 @@ function DashboardContent() {
return (
<div>
{/* Retend sees both of these Async Cells and tells the boundary to wait for both! */}
{If(userData, { true: (u) => <h2>{u.name}'s Dashboard</h2> })}
{If(userData, (u) => (
<h2>{u.name}'s Dashboard</h2>
))}
{For(postsData, (p) => (
<p>{p.title}</p>
))}
Expand Down
14 changes: 6 additions & 8 deletions docs/source/routes/docs/DocsSidebarChrome.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,12 @@ export function MobileOverlay(props: MobileOverlayProps) {

return (
<>
{If(isOpen, {
true: () => (
<div
class="fixed inset-0 z-40 bg-black/50 lg:hidden"
onClick={toggle}
/>
),
})}
{If(isOpen, () => (
<div
class="fixed inset-0 z-40 bg-black/50 lg:hidden"
onClick={toggle}
/>
))}
</>
);
}
1 change: 0 additions & 1 deletion experiments/unique-test/source/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ const Box = createUnique(() => {

return (
<UniqueTransition
topLayer
transitionDuration="2000ms"
transitionTimingFunction="ease-in-out"
>
Expand Down
79 changes: 79 additions & 0 deletions packages/retend-oxlint-plugin/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,48 @@ function unwrapExpression(node) {
return current;
}

function getStaticPropertyName(property) {
if (property.type !== 'Property') {
return null;
}

if (property.key.type === 'Identifier') {
return property.key.name;
}

if (isStaticStringLiteral(property.key)) {
return property.key.value;
}

return null;
}

function isTrueOnlyConditionObject(node) {
if (node?.type !== 'ObjectExpression') {
return false;
}

let hasTrueBranch = false;
let hasFalseBranch = false;

for (const property of node.properties) {
const propertyName = getStaticPropertyName(property);
if (propertyName === null) {
return false;
}

if (propertyName === 'true') {
hasTrueBranch = true;
}

if (propertyName === 'false') {
hasFalseBranch = true;
}
}

return hasTrueBranch && !hasFalseBranch;
}

function isProviderElementName(node) {
if (node.type === 'JSXIdentifier') {
return node.name === 'Provider';
Expand Down Expand Up @@ -1115,6 +1157,42 @@ const noDerivedInJsx = {
},
};

const noIfThreeArgs = {
meta: {
docs: {
description: 'disallow legacy or noisy If() branch syntax',
},
schema: [],
messages: {
threeArgs:
'Do not pass a third argument to `If()`. Use `If(value, trueFn)` for a true-only branch, `If(value, { false: falseFn })` for a false-only branch, or `If(value, { true: trueFn, false: falseFn })` when both branches are present.',
trueOnlyObject:
'Do not use an object bag for a true-only `If()` branch. Pass the render function directly as the second argument: `If(value, trueFn)`.',
},
},
createOnce(context) {
return {
CallExpression(node) {
if (!isNamedCall(node, 'If')) {
return;
}

if (node.arguments.length >= 3) {
context.report({ node: node.arguments[2], messageId: 'threeArgs' });
return;
}

const branchArgument = unwrapExpression(node.arguments[1]);
if (!isTrueOnlyConditionObject(branchArgument)) {
return;
}

context.report({ node: branchArgument, messageId: 'trueOnlyObject' });
},
};
},
};

const noJsxControlFlow = {
meta: {
docs: {
Expand Down Expand Up @@ -2219,6 +2297,7 @@ const plugin = {
'no-get-in-derived-async': noGetInDerivedAsync,
'no-get-in-jsx': noGetInJsx,
'no-derived-in-jsx': noDerivedInJsx,
'no-if-three-args': noIfThreeArgs,
'no-jsx-control-flow': noJsxControlFlow,
'no-jsx-map': noJsxMap,
'no-listen-in-onsetup': noListenInOnSetup,
Expand Down
1 change: 1 addition & 0 deletions packages/retend-oxlint-plugin/recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const rules = {
'retend/no-derived-in-jsx': 'error',
'retend/no-get-in-derived-async': 'error',
'retend/no-get-in-jsx': 'error',
'retend/no-if-three-args': 'error',
'retend/no-inline-object-type': 'error',
'retend/no-jsx-control-flow': 'error',
'retend/no-jsx-map': 'error',
Expand Down
1 change: 1 addition & 0 deletions packages/retend-oxlint-plugin/recommended.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"retend/no-derived-in-jsx": "error",
"retend/no-get-in-derived-async": "error",
"retend/no-get-in-jsx": "error",
"retend/no-if-three-args": "error",
"retend/no-inline-object-type": "error",
"retend/no-jsx-control-flow": "error",
"retend/no-jsx-map": "error",
Expand Down
10 changes: 8 additions & 2 deletions packages/retend-server/source/v-dom/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,15 @@ export class VNode extends EventTarget {
}
}

for (const node of newNodes) {
if (node !== this) node.remove();
}
const index = parentNode.childNodes.indexOf(this);
parentNode.childNodes.splice(index, 1, ...newNodes);
for (const node of newNodes) {
node.parentNode = this.parentNode;
node.parentNode = parentNode;
}
this.parentNode = null;
if (!newNodes.includes(this)) this.parentNode = null;
}

/** @param {(string | VNode)[]} nodes */
Expand All @@ -120,6 +123,9 @@ export class VNode extends EventTarget {
? n
: ownerDocument.createTextNode(n)
);
for (const node of newNodes) {
node.remove();
}
for (const node of this.childNodes) {
node.parentNode = null;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/retend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"build": "node scripts/build.js"
},
"dependencies": {
"@adbl/cells": "https://pkg.pr.new/adebola-io/cells/@adbl/cells@9fb5142",
"@adbl/cells": "^0.0.24",
"csstype": "^3.1.3"
},
"devDependencies": {
Expand Down
6 changes: 4 additions & 2 deletions packages/retend/source/library/scope.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ class EffectNode {

async activate() {
if (!this.#enabled || this.#active) return;
await new Promise((resolve) => setTimeout(resolve));
await new Promise((resolve) => {
queueMicrotask(() => resolve(undefined));
});
await this.#runActivateFns();
this.renderer?.host.dispatchEvent(new Event('retend:activate'));
Comment on lines 139 to 145

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Reentrant activation duplicates setup 🐞 Bug ≡ Correctness

EffectNode.activate() now defers via queueMicrotask before running #runActivateFns(), but
#runActivateFns() awaits each SetupFn and only sets #active after the loop, so a second activate()
call can enter while the first is mid-await and run the same setup effects again. This is reachable
because multiple call sites invoke node.activate() without awaiting it (e.g., If/For updates), so
repeated activations in the same tick can overlap and duplicate side effects like event
listeners/timers.
Agent Prompt
## Issue description
`EffectNode.activate()` can be invoked multiple times before the node becomes active. After this PR, activation is microtask-based, which allows concurrent/overlapping activations while `#runActivateFns()` is mid-`await`, causing setup effects to execute more than once.

## Issue Context
- `#runActivateFns()` awaits each `SetupFn` and only sets `#active` after all setup functions complete.
- Several call sites call `node.activate()` without `await`, so repeated updates in the same tick can trigger multiple activations.

## Fix Focus Areas
- packages/retend/source/library/scope.js[120-146]
- packages/retend/source/library/if.js[149-158]
- packages/retend/source/library/for.js[237-242]

## Implementation direction
- Add a single-flight guard in `EffectNode` (e.g., `#activationPromise` or `#activating` flag).
- In `activate()`, if an activation is already in progress, return/await the same promise.
- Ensure `retend:activate` is dispatched once per activation and only after setup execution.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}
Expand Down Expand Up @@ -538,7 +540,7 @@ export async function runPendingSetupEffects() {
renderer?.observer?.flush();
node.enable();
await node.activate();
await new Promise((resolve) => setTimeout(resolve));
await new Promise((resolve) => queueMicrotask(() => resolve(undefined)));
}

/** @type {Scope<__HMR_UpdatableFn[]>} */
Expand Down
11 changes: 7 additions & 4 deletions packages/retend/source/router/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,7 @@ export class Router extends EventTarget {
if (!this.#assertNotLocked(path)) return;
this.#isNavigating = true;
try {
await this.#load({ rawPath: path, replace: true });
await this.#load({ rawPath: path, navigate: true, replace: true });
} finally {
this.#isNavigating = false;
}
Expand Down Expand Up @@ -650,9 +650,12 @@ export class Router extends EventTarget {
if (this.#isNavigating) return;
const window = /** @type {Window} */ (event.currentTarget);
this.#isNavigating = true;
const path = getFullPath(window);
await this.#load({ rawPath: path, navigate: false });
this.#isNavigating = false;
try {
const path = getFullPath(window);
await this.#load({ rawPath: path, navigate: false });
} finally {
this.#isNavigating = false;
}
};

/** @param {Window} window */
Expand Down
Loading
Loading