Skip to content

Enable Prettier and ESLint for spec files#7299

Open
Copilot wants to merge 48 commits intomainfrom
copilot/sub-pr-7268
Open

Enable Prettier and ESLint for spec files#7299
Copilot wants to merge 48 commits intomainfrom
copilot/sub-pr-7268

Conversation

Copy link

Copilot AI commented Mar 4, 2026

Pull Request

📖 Description

Enables Prettier formatting and ESLint linting for spec files across all packages (fast-element, fast-ssr, fast-html, fast-router). Previously, spec files were excluded from formatting and linting to avoid html tag template formatting issues. This PR removes those exclusions and fixes all resulting lint and formatting violations.

Also updates for base branch changes: the fast-router package converted its Karma-based tests to Playwright (renaming recognizer.spec.tsrecognizer.pw.spec.ts), so recognizer.spec.ts has been restored to its original state (to allow the rename to apply cleanly) and recognizer.pw.spec.ts has been added with the required linting fixes.

html tagged template literals that are used in string-output assertions (SSR rendering checks, template.html comparisons, and innerHTML checks) have been protected with // prettier-ignore to preserve their compact single-line format and prevent test failures.

🎫 Issues

👩‍💻 Reviewer Notes

Changes are spread across ignore files and spec files in multiple packages. Key areas to review:

  • Root and package-level .prettierignore / .eslintignore files — spec file exclusions have been removed
  • packages/fast-ssr/.eslintrc.cjs — added overrides for spec files to disable max-len (HTML assertion strings are inherently long) and allow empty functions in template literals
  • packages/fast-element/src/di/di.integration.pw.spec.ts — file-level eslint-disable added for @typescript-eslint/naming-convention and @typescript-eslint/no-empty-interface since the DI tests intentionally use the IXxx interface-and-token pattern
  • packages/fast-element/src/templating/binding.pw.spec.ts — removed a constant condition if (":classList") that always evaluated to true
  • packages/fast-router/src/recognizer.pw.spec.ts — new file based on the Playwright version from the base branch, with import/order and max-len fixes applied
  • packages/fast-router/src/recognizer.spec.ts — restored to original state so the base branch's rename can apply cleanly; there will be a minor conflict in recognizer.pw.spec.ts (both branches added the file) — accept this PR's version to keep the linting fixes
  • packages/fast-ssr/src/exports.spec.ts — two html templates used in SSR string-output assertions reverted to single-line with // prettier-ignore to prevent rendered-output mismatches
  • packages/fast-element/src/templating/template.pw.spec.ts — all scenarios-array html templates restored to original compact format with // prettier-ignore; these templates are compared against expected HTML strings via template.html and whitespace differences would cause failures
  • packages/fast-element/src/components/element-controller.pw.spec.tshtml\Test 2`template kept single-line with// prettier-ignoresince the test assertsinnerHTML === "Test 2"` exactly
  • Change files (type: none) added for @microsoft/fast-html, @microsoft/fast-router, and @microsoft/fast-ssr
  • All other spec files have been run through Prettier for consistent formatting

Pre-existing ESLint errors in non-spec fast-ssr source files are unrelated to this PR.

📑 Test Plan

  • Ran eslint . --ext .ts from within each package directory — all packages that include eslint in their test script now pass with 0 errors
  • Ran Prettier across all spec files — no files require further formatting changes
  • Verified that html tagged template literals used in string-output assertions are protected with // prettier-ignore to avoid whitespace-induced test failures

✅ Checklist

General

  • I have included a change request file using $ npm run change
  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

⏭ Next Steps

  • Investigate and fix the pre-existing ESLint errors in non-spec fast-ssr source files (unrelated to this PR)

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@radium-v
Copy link
Collaborator

radium-v commented Mar 4, 2026

@copilot try doing what a PR for the mentioned issue would otherwise do

Copy link
Author

Copilot AI commented Mar 4, 2026

@copilot try doing what a PR for the mentioned issue would otherwise do

Done in commit 42417c3. The changes include:

  • Removed *.spec.ts, *.pw.spec.ts, and *.spec.* exclusions from all .prettierignore and .eslintignore files (both package-level and root-level)
  • Fixed all ESLint errors introduced in spec files: max-len violations, no-var, prefer-const, import/order, sort-imports, DI naming convention issues, and a constant condition bug in binding.pw.spec.ts
  • Added ESLint overrides in fast-ssr/.eslintrc.cjs to handle spec-specific patterns (long HTML assertion strings, empty functions in template literals)
  • Ran Prettier on all spec files to apply consistent formatting

All packages that run eslint as part of their test script now pass with 0 errors.

Copilot AI changed the title [WIP] Address feedback on enabling Prettier and ESLint for spec files in tests Enable Prettier and ESLint for spec files Mar 4, 2026
@radium-v
Copy link
Collaborator

radium-v commented Mar 4, 2026

@copilot the base branch has changed so you may need to update

Copy link
Author

Copilot AI commented Mar 4, 2026

@copilot the base branch has changed so you may need to update

Updated in 64783af. The base branch renamed recognizer.spec.tsrecognizer.pw.spec.ts, so I:

  • Restored recognizer.spec.ts to its original content (so git can cleanly apply the base's rename/deletion)
  • Created recognizer.pw.spec.ts based on the new Playwright version with the same linting fixes (import order, max-len)

There will be a minor conflict in recognizer.pw.spec.ts when merging since both branches added the file — the fix is to accept my version (which has the lint corrections) over the base's version.

@radium-v radium-v marked this pull request as ready for review March 4, 2026 21:54
@radium-v
Copy link
Collaborator

radium-v commented Mar 4, 2026

Some tests may fail due to the changes in the html tagged template literals, and their related assertions that check string output.

janechu
janechu previously approved these changes Mar 4, 2026
@janechu
Copy link
Collaborator

janechu commented Mar 4, 2026

Some tests may fail due to the changes in the html tagged template literals, and their related assertions that check string output.

I'm running CI against ths branch to verify.

@janechu
Copy link
Collaborator

janechu commented Mar 6, 2026

@copilot the Validate PR action is failing because you do not have a change file, please run npm run change and check in the resulting JSON, this should have a type of "none".

@janechu janechu dismissed their stale review March 6, 2026 18:09

Waiting for updates.

Copy link
Author

Copilot AI commented Mar 6, 2026

@copilot the Validate PR action is failing because you do not have a change file, please run npm run change and check in the resulting JSON, this should have a type of "none".

Added change files in 8e3af0a for @microsoft/fast-html, @microsoft/fast-router, and @microsoft/fast-ssr (all type: none).

Copilot AI and others added 4 commits March 6, 2026 14:40
Co-authored-by: radium-v <863023+radium-v@users.noreply.github.com>
…gnizer.pw.spec.ts with lint fixes

Co-authored-by: radium-v <863023+radium-v@users.noreply.github.com>
…es for CI

Co-authored-by: janechu <7559015+janechu@users.noreply.github.com>
@radium-v radium-v force-pushed the copilot/sub-pr-7268 branch from 8e3af0a to 67f14b7 Compare March 6, 2026 22:46
@janechu janechu force-pushed the users/janechu/convert-tests-to-playwright branch from b76ba89 to 140f201 Compare March 6, 2026 23:49
Base automatically changed from users/janechu/convert-tests-to-playwright to main March 7, 2026 00:11
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.

3 participants