Repository navigation
Fix @astrojs/cloudflare dependency scan incorrectly matching import.meta and import() as static imports - #18145
Conversation
…port hoisting regex
🦋 Changeset detectedLatest commit: c1e7a92 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Merge Requirement Assessment
Status: satisfied
- Fix
@astrojs/cloudflaredependency scan incorrectly matchingimport.metaandimport()as static imports -isImportExpressionreturns true forimport.meta/import(...)lines (allowing whitespace and both/* *////comments between theimporttoken and.or(), so thewrapFrontmatterreplace callback leaves those expressions inside the__astro__()body instead of hoisting them. The added unit tests cover the Prettier-wrapped, line-leadingimport.metapattern, dynamicimport()calls, and both comment-separated variants described in the issue.
Review Status
Verdict: ready to merge based on static review
Context: PR #18145 (closes #18144)
Scope: Full PR diff (base 3f3d580b83b0cb9f14d451b86dab547ccffe4eab → head c1e7a92f0444b26b70c38339ce092a4bcffec259) — packages/integrations/cloudflare/src/rolldown-plugin-astro-frontmatter.ts, packages/integrations/cloudflare/test/units/wrap-frontmatter.test.ts, .changeset/hot-places-ask.md.
Changeset: present and covers @astrojs/cloudflare.
Limitations: none.
Validation: Static review only; no project code, tests, builds, or checks were run.
GitHub context: collected.
Git scope: PR base/head verified via GitHub read-only tools.
This review was made by an LLM. The analysis may be wrong, and reports might be incorrect.
Factory AdversaryPurple selected red, not Blue. Confidence: medium. This is a bug fix in `@astrojs/cloudflare`'s frontmatter wrapping transform used during dependency scanning. The contract is to stop treating `import.meta` property access and dynamic `import()` calls as static import declarations (so they are not hoisted out of the `__astro__()` wrapper), while continuing to hoist real static imports and including focused regression tests plus a patch changeset. Red and blue differ in implementation: red uses a post-match `isImportExpression` helper that skips whitespace/comments before checking for `.` or `(`, while blue adds a negative lookahead `(?!\s*[.(])` to the same regex. Both pass their own new unit tests for line-leading `import.meta` and `import()`, but blue fails on comment-separated variants (e.g., `import /* c */ .meta.url` and `import // c\n ("./mod")`), leaving the wrapper invalid. Red also covers those cases in tests and implementation. Because correctness is the first Astro comparison criterion, red is the better solution. No alternative pull request was created. Disclosure: The alternative and assessment were generated by AI agents. |
Changes
(?!\s*[.(])after\binIMPORT_STMT_REinrolldown-plugin-astro-frontmatter.ts. Previously, the word boundary\bmatched betweenimportand.or((both non-word characters), causing Prettier-wrapped expressions likeimport.meta.env.BASE_URL…at the start of a line to be incorrectly hoisted out of the__astro__()wrapper. This left the preceding assignment incomplete, producing a parse error that silently failed the entire dependency pre-bundling scan project-wide.import.metaand dynamicimport()expressions.Testing
packages/integrations/cloudflare/test/units/wrap-frontmatter.test.ts: one covering a line-leadingimport.metaexpression (the Prettier-wrapped pattern), and one covering a dynamicimport()call — both verifying the expressions remain inside the__astro__()body rather than being hoisted.Docs
Closes #18144