Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(linter): Add more tests for ESLintConfig #2284

Merged
merged 13 commits into from
Feb 5, 2024
Merged

chore(linter): Add more tests for ESLintConfig #2284

merged 13 commits into from
Feb 5, 2024

Conversation

leaysgur
Copy link
Contributor

@leaysgur leaysgur commented Feb 3, 2024

Before trying #2258 , I'd like to prevent regression. 🦺

Overview

  • Rename ESLintConfig::new(path) -> from_file(path)
  • Split from_file() implementation into 2 parts
    • Parse path, strip json comment, check .json ext part
    • from_value(): Read +parse JSON contents part
      • ☝🏻used in tests
  • Add tests for parsing rules, settings, env

TODOs found, for next PR

  • rules parser should handle "no-debugger": 1 form
  • settings.xxx_components should go under settings.react.

Notes

Copy link

netlify bot commented Feb 3, 2024

👷 Deploy Preview for oxc processing.

Name Link
🔨 Latest commit 661d67b
🔍 Latest deploy log https://app.netlify.com/sites/oxc/deploys/65c06ed63364b30007f29bb8

@github-actions github-actions bot added the A-linter Area - Linter label Feb 3, 2024
Copy link

codspeed-hq bot commented Feb 3, 2024

CodSpeed Performance Report

Merging #2284 will not alter performance

Comparing issue_2258-pre (9411ac2) with main (41d1876)

Summary

✅ 27 untouched benchmarks

@leaysgur leaysgur marked this pull request as ready for review February 5, 2024 06:41
Copy link
Member

@Boshen Boshen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for working on this, I hope you enjoyed some Rust :-)

@Boshen Boshen merged commit b27079c into main Feb 5, 2024
19 checks passed
@Boshen Boshen deleted the issue_2258-pre branch February 5, 2024 12:42
IWANABETHATGUY pushed a commit to IWANABETHATGUY/oxc that referenced this pull request May 29, 2024
Before trying  oxc-project#2258 , I'd like to prevent regression. 🦺 

### Overview

- Rename `ESLintConfig::new(path)` -> `from_file(path)`
- Split `from_file()` implementation into 2 parts
  - Parse path, strip json comment, check `.json` ext part
  - `from_value()`: Read +parse JSON contents part
    - ☝🏻used in tests
- Add tests for parsing rules, settings, env

### TODOs found, for next PR
- `rules` parser should handle `"no-debugger": 1` form
- `settings.xxx_components` should go under `settings.react.`

### Notes

- `rules`'s type
  - https://github.com/eslint/eslint/blob/main/lib/shared/types.js#L12
- `settings`'s type is `Object` 😅 
  - https://github.com/eslint/eslint/blob/main/lib/shared/types.js#L53
  - and its usage is extended by each plugin
-
https://github.com/jsx-eslint/eslint-plugin-react?tab=readme-ov-file#configuration-legacy-eslintrc-
-
https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/?tab=readme-ov-file#configurations
-
https://nextjs.org/docs/pages/building-your-application/configuring/eslint#eslint-plugin
- `env`'s type is just a `Record<string, boolean>`
  - https://github.com/eslint/eslint/blob/main/lib/shared/types.js#L40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linter Area - Linter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants