Skip to content

build: Replace fragile string-based comment stripping with AST-based approach #770

@grypez

Description

@grypez

Problem

The current stripCommentsPlugin in packages/cli/src/vite/strip-comments-plugin.ts uses character-by-character string parsing to identify and remove comments. This approach is fragile because it doesn't use an authoritative model of JavaScript syntax:

  • Regex literals: /\/\// looks like a comment start to the string parser
  • Template literals: Complex escape sequences and interpolations can confuse the parser
  • Edge cases: The string parser guesses at syntax rather than understanding it

We need a solution that interacts with an authoritative AST model that definitively identifies comment nodes, then removes them.

Context

SES (Secure ECMAScript) rejects code containing import( patterns, even when they appear in comments. The comment stripping plugin exists to sanitize bundled code before it's loaded in a SES Compartment.

Options

Option A: Acorn + magic-string

Use Acorn (already a transitive dependency via vite/rollup) to parse the code and collect comment positions, then use magic-string to remove those exact ranges.

import { parse, type Comment } from 'acorn';
import MagicString from 'magic-string';

export function stripCommentsPlugin(): Plugin {
  return {
    name: 'strip-comments',
    renderChunk(code) {
      const comments: Comment[] = [];

      parse(code, {
        ecmaVersion: 'latest',
        sourceType: 'module',
        onComment: comments,  // Acorn populates this with { start, end } for each comment
      });

      if (comments.length === 0) return null;

      const s = new MagicString(code);
      for (const comment of comments) {
        s.remove(comment.start, comment.end);
      }
      return s.toString();
    },
  };
}

Pros:

  • Acorn is the authoritative JavaScript parser - it definitively knows what is a comment
  • Both acorn and magic-string are already in the dependency tree (transitive via vite)
  • Well-tested, mature libraries

Cons:

  • Re-parses the already-bundled code (some overhead)
  • Acorn is a separate parser from what Rollup uses internally

Option B: Vite 8.0 / Rolldown

Vite 8.0 (currently in beta) replaces Rollup with Rolldown, a Rust-based bundler that exposes its AST directly to the JavaScript plugin API. This would allow us to hook into the same AST the bundler uses.

Pros:

  • Single source of truth - use the bundler's own AST
  • No re-parsing overhead
  • Rolldown is designed for this use case

Cons:

  • Vite 8.0 is still in beta
  • Requires investigating the Rolldown plugin API for comment access
  • May require waiting for stable release

Acceptance Criteria

  • Comments (both // and /* */) are stripped from bundled vat code
  • Code behavior is preserved exactly after bundling and comment removal
  • The solution uses an authoritative AST model, not string heuristics
  • Edge cases are handled correctly:
    • Strings containing // or /* patterns
    • Regex literals like /\/\//
    • Template literals with complex content

Verification

  1. Existing tests pass: yarn workspace @ocap/kernel-test test:dev
  2. Bundles load correctly in SES Compartments
  3. No import( patterns remain in comments (SES compatibility)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions