host: Remove Monaco Percy snapshots and add lint rule#3961
host: Remove Monaco Percy snapshots and add lint rule#3961
Conversation
Preview deployments |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 233b4d5713
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fix(fixer) { | ||
| return fixer.replaceText( | ||
| node, | ||
| "import { percySnapshot } from '@cardstack/host/tests/helpers';", |
There was a problem hiding this comment.
Make the autofix safe for non-percySnapshot imports
The rule applies to any ImportDeclaration from @percy/ember, but the autofix always replaces the entire import with import { percySnapshot } .... If a file imports other exports (e.g., import { foo } from '@percy/ember' or import * as percy ...), running eslint --fix will silently drop those imports and change the binding, leading to runtime errors where the original symbols are used. Consider limiting the fixer to cases that actually import percySnapshot (default or named), or remove the autofix and only report.
Useful? React with 👍 / 👎.
This removes snapshots that cause common false negatives, like here. We haven’t been able to find a way to stabilise these and they’re a constant noise. Other snapshots may qualify, we can remove them as we see them.
This also adds a custom lint rule that enforces the use of the overridden
percySnapshot. You can see it warning here about the incorrect import, which is now fixed.