Skip to content

@remotion/studio-server: Cache AST node paths to survive stale source maps#6942

Merged
JonnyBurger merged 2 commits intomainfrom
fix/node-path-cache-stale-sourcemaps
Mar 31, 2026
Merged

@remotion/studio-server: Cache AST node paths to survive stale source maps#6942
JonnyBurger merged 2 commits intomainfrom
fix/node-path-cache-stale-sourcemaps

Conversation

@JonnyBurger
Copy link
Copy Markdown
Member

Summary

  • Adds a server-side cache mapping (fileName, line, column) → AST nodePath to handle stale source maps after suppressed webpack rebuilds
  • When prettier reformats a file (e.g. wrapping a return in parentheses), JSX tags shift lines. The stale source map still resolves to the old line, but the cached nodePath remains valid since the AST structure is unchanged
  • Cache is cleared on compiler.hooks.done when webpack rebuilds and source maps become fresh again

Problem

// Before — source map says <Video> is on line 22:
export const Component = () => {
  return <Video src={src} />;      // line 22
};

// After adding style={{}} + prettier wrapping — tag moves to line 23:
export const Component = () => {
  return (
    <Video src={src} style={{}} />  // now line 23
  );
};

On reload, the stale source map still resolves to line 22, but lineColumnToNodePath(ast, 22) finds nothing → subscribe-to-sequence-props fails.

Solution

  1. On first successful subscription: cache (file, line, col) → nodePath
  2. On subsequent requests with stale coordinates: reuse cached nodePath (verified against current AST)
  3. On webpack rebuild: clear cache (source maps are fresh, line-based lookup works)

Test plan

  • Added e2e test in packages/example/e2e/node-path-cache.test.mts that verifies the cache handles line shifts
  • Existing visual controls and undo-redo e2e tests still pass
  • Manual test: open studio, edit a component to trigger prettier reformatting, reload — sequence props controls still work

🤖 Generated with Claude Code

…ce maps

When the studio modifies a file and suppresses the webpack rebuild, the
source map becomes stale. If prettier then reformats the file (e.g. wrapping
a return in parentheses), JSX tags shift to different lines. On reload,
the stale source map still resolves to the old line number, causing
subscribe-to-sequence-props to fail.

This adds a server-side cache mapping (fileName, line, column) → AST
nodePath. On the first successful subscription, the resolved nodePath is
cached. Subsequent requests with the same stale coordinates reuse the
cached nodePath (verified against the current AST). The cache is cleared
when webpack rebuilds via compiler.hooks.done, at which point source maps
are fresh and line-based lookup works again.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@pullfrog
Copy link
Copy Markdown
Contributor

pullfrog bot commented Mar 31, 2026

Reviewed PR #6942 — no actionable issues found. The cache-and-verify pattern is correct: cached nodePath is validated against the current AST on every hit, stale entries gracefully fall back to line-based lookup, and compiler.hooks.done clears the cache at the right lifecycle point. The column parameter was already in the shared request type and sent by the client — this PR simply wires it through on the server side for use as a cache-key differentiator.

Task list (6/6 completed)
  • Checkout PR and read diff
  • Read changed files and trace data flow
  • Investigate high-risk areas (cache correctness, invalidation, concurrency)
  • Run impact analysis (grep for stale references)
  • Draft and self-critique review comments
  • Submit review

Pullfrog  | View workflow run | Triggered by Pullfrog | Using Big Pickle (free) | 𝕏

@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Mar 31, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
bugs Ready Ready Preview, Comment Mar 31, 2026 1:10pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
remotion Skipped Skipped Mar 31, 2026 1:10pm

Request Review

The `handleRequest` handler wraps all API responses in `{success, data}`,
so test assertions need to access `result.data.canUpdate` instead of
`result.canUpdate`.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel vercel bot temporarily deployed to Preview – remotion March 31, 2026 13:09 Inactive
@JonnyBurger JonnyBurger enabled auto-merge March 31, 2026 13:14
@JonnyBurger JonnyBurger merged commit bc9f5ec into main Mar 31, 2026
17 of 18 checks passed
@JonnyBurger JonnyBurger deleted the fix/node-path-cache-stale-sourcemaps branch March 31, 2026 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant