Skip to content

fix(core): Resolve host data paths once per expression evaluation - #42571

Merged
looorent merged 3 commits into
masterfrom
cet-540-a6-find-why-quickjs-times-out-on-simple-expressions-in-the
Oct 29, 2026
Merged

looorent merged 3 commits into
masterfrom
cet-540-a6-find-why-quickjs-times-out-on-simple-expressions-in-the

Conversation

@looorent

@looorent looorent commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Reading an array from an expression parameter could repeat the same calculation for every element and cause QuickJS to time out. This PR reuses resolved host values within each evaluation.

The fix applies to both QuickJS and isolated-vm.

Why

Consider an ids parameter that contains:

{{ $input.all().map(item => item.json.id) }}

Reading {{ $parameter.ids }} looks simple. But the runtime reads array elements on demand. Each read used to resolve the path from the root, which could run the entire map() again.

The array is now resolved once and reused while its elements are read.

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/CET-540

Review / Merge checklist

  • I have seen this code, I have run this code, and I take responsibility for this code.
  • PR title and summary are descriptive. (conventions)
  • Tests included.
  • PR Labeled with Backport to Beta, Backport to Stable, or Backport to v1 (if the PR is an urgent fix that needs to be backported)

View guided diff Turn on auto-fix

A parameter value can hold an expression, so reading it can evaluate a
nested expression. Paths such as `$node["A"].parameter.ids` and
`$item(0).$parameter.ids` reach that value below the first two steps,
so the resolver read it again for each array element.

Cache the path down to the parameter value for `$parameter` and
`$node[name].parameter`, also under `$item(i)`. Keep the default depth
for other paths.

@cubic-dev-ai cubic-dev-ai Bot left a comment

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.

No issues found across 8 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Shadow auto-approve: would auto-approve. Bounded performance fix: caches resolved host data path prefixes once per expression evaluation in QuickJS and isolated-vm, avoiding repeated re-evaluation of parameter expressions; covered by unit and integration tests.

View guided diff | Turn on auto-fix | Re-trigger cubic

@looorent
looorent marked this pull request as ready for review October 29, 2026 12:50
@looorent
looorent force-pushed the cet-540-a6-find-why-quickjs-times-out-on-simple-expressions-in-the branch from 5bf827c to 5736079 Compare October 29, 2026 12:50
@codecov

codecov Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Bundle Report

Changes will increase total bundle size by 383 bytes (0.0%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
editor-ui-esm 61.61MB 383 bytes (0.0%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: editor-ui-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
assets/app-*.js 383 bytes 9.15MB 0.0%
assets/worker-*.js 3.41MB 3.43MB 17518.09% ⚠️
assets/worker-*.js -3.41MB 19.46kB -99.43%
assets/src-*.js -74 bytes 64 bytes -53.62%
assets/src-*.js 74 bytes 138 bytes 115.62% ⚠️

@codecov

codecov Bot commented Oct 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@n8n-assistant

n8n-assistant Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

PR review overview

Based on ownership of the 8 changed files in this PR:

Ownership Files owned Share Source code Test files Misc
@n8n-io/core-execution 8 100% +141 / -157 +471 / -3 +0 / -0
Total 8 100% +141 / -157 +471 / -3 +0 / -0

Co-Authored-By: Kimi K3.1 Pro <noreply@moonshot.ai>

@despairblue despairblue left a comment

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.

added more tests, rests looks nice, good work

@cubic-dev-ai cubic-dev-ai Bot left a comment

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.

0 issues found across 2 files (changes from recent commits).

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Shadow auto-approve: would not auto-approve. Auto-approval skipped because cubic reviewed only this push, not the earlier force-push. Comment @cubic review to review the whole pull request.

View guided diff | Turn on auto-fix | Re-trigger cubic

@looorent
looorent enabled auto-merge October 29, 2026 14:50
@looorent
looorent added this pull request to the merge queue Oct 29, 2026
Merged via the queue into master with commit d50a446 Oct 29, 2026
81 checks passed
@looorent
looorent deleted the cet-540-a6-find-why-quickjs-times-out-on-simple-expressions-in-the branch October 29, 2026 15:35
@n8n-assistant

n8n-assistant Bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Created backport PR for release-candidate/2.46.x:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-42571-to-release-candidate/2.46.x
git worktree add --checkout .worktree/backport-42571-to-release-candidate/2.46.x backport-42571-to-release-candidate/2.46.x
cd .worktree/backport-42571-to-release-candidate/2.46.x
git reset --hard HEAD^
git cherry-pick -x d50a44698bfb08f368dad62bb9fde7e2947da396
git push --force-with-lease

@n8n-assistant

n8n-assistant Bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Created backport PR for release-candidate/2.45.x:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-42571-to-release-candidate/2.45.x
git worktree add --checkout .worktree/backport-42571-to-release-candidate/2.45.x backport-42571-to-release-candidate/2.45.x
cd .worktree/backport-42571-to-release-candidate/2.45.x
git reset --hard HEAD^
git cherry-pick -x d50a44698bfb08f368dad62bb9fde7e2947da396
git push --force-with-lease

@n8n-assistant

n8n-assistant Bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Created backport PR for release-candidate/2.46.x:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-42571-to-release-candidate/2.46.x
git worktree add --checkout .worktree/backport-42571-to-release-candidate/2.46.x backport-42571-to-release-candidate/2.46.x
cd .worktree/backport-42571-to-release-candidate/2.46.x
git reset --hard HEAD^
git cherry-pick -x d50a44698bfb08f368dad62bb9fde7e2947da396
git push --force-with-lease

looorent added a commit that referenced this pull request Oct 29, 2026
…2571)

Co-authored-by: Danny Martini <danny@n8n.io>
Co-authored-by: Kimi K3.1 Pro <noreply@moonshot.ai>
(cherry picked from commit d50a446)
@n8n-assistant n8n-assistant Bot mentioned this pull request Oct 30, 2026
3 of 6 tasks
looorent added a commit that referenced this pull request Oct 30, 2026
…2571)

Co-authored-by: Danny Martini <danny@n8n.io>
Co-authored-by: Kimi K3.1 Pro <noreply@moonshot.ai>
(cherry picked from commit d50a446)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants