fix: externalize prettier/plugins/estree to prevent comment formatting crash#425
Merged
evoactivity merged 2 commits intoember-tooling:mainfrom Mar 31, 2026
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts the Vite/Rollup build configuration so Prettier’s estree/babel plugin modules are truly externalized (matching the source import specifiers), preventing runtime crashes caused by bundling an outdated Prettier printer implementation when users run newer Prettier versions.
Changes:
- Fix Rollup
externalentries to matchprettier/plugins/estreeandprettier/plugins/babelimport paths (no.jsextension). - Add a
.gtsregression case covering “comment +<template>” formatting. - Update Vitest snapshots for the new test case across relevant config suites.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.mjs | Fixes externalization specifiers so Prettier plugin modules aren’t bundled unintentionally. |
| tests/cases/gts/comment-with-template.gts | Adds regression input reproducing the crash scenario. |
| tests/unit-tests/snapshots/format.test.ts.snap | Records expected formatting output for the new case in the default suite. |
| tests/unit-tests/config/snapshots/semi-false.test.ts.snap | Records expected formatting output for the new case under semi: false. |
| tests/unit-tests/config/snapshots/template-export-default.test.ts.snap | Records expected formatting output for the new case under templateExportDefault: true. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The vite config listed `prettier/plugins/estree.js` as external, but the source imports `prettier/plugins/estree` (no .js extension). This mismatch caused vite to bundle prettier's estree printer into the plugin instead of keeping it external. When the user installs a newer version of prettier (e.g. 3.6.2), the plugin still uses the old bundled estree code whose `canAttachComment` function expects 2 arguments, while prettier 3.6+'s core calls it with 1 argument. This causes a crash: TypeError: undefined is not iterable (cannot read property Symbol(Symbol.iterator)) when formatting any .gts file that contains both a comment and a <template> tag. The fix removes the .js extension from the external config so it matches the actual import paths. Also adds a regression test case. Closes ember-tooling#424
lifeart
force-pushed
the
fix-estree-external
branch
from
1719d02 to
2cbbfe5
Compare
…ests Fix lint issues in build-externals test (import style, naming conventions, encoding identifier) and add missing `pnpm build` step to test-prettier-versions CI job so the dist file exists when the build-externals test runs. Also make assertions resilient to both CJS and ESM output formats. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
lifeart
commented
Mar 31, 2026
| const distribution = readFileSync(distributionPath, 'utf8'); | ||
|
|
||
| expect(distribution).toMatch( | ||
| /require\(["']prettier\/plugins\/babel["']\)|from\s+["']prettier\/plugins\/babel["']/, |
Contributor
Author
There was a problem hiding this comment.
covering cjs/esm cases (esm - for future use)
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #424
The vite config listed
prettier/plugins/estree.jsandprettier/plugins/babel.jsas external, but the source importsprettier/plugins/estreeandprettier/plugins/babel(no.jsextension). This mismatch caused vite to bundle prettier's estree printer into the plugin instead of keeping it external.When the user installs a newer version of prettier (e.g. 3.6.2), the plugin still uses the old bundled estree code whose
canAttachCommentfunction expects 2 arguments (node, ancestors), while prettier 3.6+'s core calls it with only 1 argument (node). This causes a crash:when formatting any
.gtsfile that contains both a comment and a<template>tag.Fix
Remove the
.jsextension from the external config entries so they match the actual import paths in the source code:This ensures prettier's estree and babel plugins are loaded from the user's installed prettier version at runtime rather than being bundled at build time.
Minimal reproduction
Test
Added
tests/cases/gts/comment-with-template.gtsregression test case.