Conversation
Flags `String(error)` in the else branch of `error instanceof Error` ternaries, where String() produces '[object Object]' for non-Error objects. Suggests using JSON.stringify() instead to preserve object structure.
Adds platform tagging (expo, web, backend) to all rules via a meta registry.
Rules without a platform tag are universal and run on all platforms.
- New `platform` config option in lintJsxCode() that takes precedence over rules/exclude
- New `getRulesForPlatform()` API to get rule names for a platform
- Platform column added to README rule tables
- Config support: `{ "platform": "expo" }` in laint.config.json
danielchen0
changed the base branch from
add-no-string-coerce-error-rule
to
main
arnavsurve
left a comment
arnavsurve
left a comment
There was a problem hiding this comment.
This is a well-structured PR that adds platform-based rule filtering and a new no-string-coerce-error rule. The architecture is clean: a separate meta.ts registry keeps platform tags decoupled from rule implementations, getRulesForPlatform() correctly unions platform-tagged rules with universal ones, and precedence logic in lintJsxCode is straightforward. Test coverage is thorough with 8 platform tests covering all three platforms, universality, reachability, and precedence, plus 6 tests for the new rule. All CI checks pass.
Two minor items noted inline -- one is a stale rule count in the README, and the other is a type ergonomics suggestion. Neither blocks merging.
🤖 Generated with Claude Code
|
|
||
| ```json | ||
| // Run all rules for a platform (platform mode) | ||
| { "platform": "expo" } |
There was a problem hiding this comment.
Bug (outside this hunk): Line 17 of the README still says "By default, all 31 rules run" but should say "all 34 rules" to match the "Available Rules (34 total)" heading and the actual count in rules/index.ts.
| */ | ||
| exclude?: boolean; | ||
| /** | ||
| * When set, runs all rules tagged for this platform plus universal rules. |
There was a problem hiding this comment.
Nit: rules is required in LintConfig, but the README shows { "platform": "expo" } (no rules field) as a valid config. This works at runtime in the CLI because loadConfig() casts with as LintConfig and platform is checked first, but it means:
- The TypeScript type doesn't match the documented JSON schema.
- Programmatic API users in platform mode must pass a meaningless
rules: [].
Consider making rules optional (with a default of [] in the implementation) or using a discriminated union. Not blocking.
2 participants
Summary
expo,web,backend) to all 34 rules via asrc/rules/meta.tsregistryplatformconfig option:{ "platform": "expo" }runs expo-tagged + universal rulesgetRulesForPlatform()export to query which rules belong to a platformTest plan