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

ci: specify legacy config for e2e eslint #6690

Closed
wants to merge 3 commits into from

Conversation

MikeMcC399
Copy link
Contributor

@MikeMcC399 MikeMcC399 commented Feb 16, 2025

What's the problem this PR addresses?

The workflow .github/workflows/e2e-eslint-workflow.yml fails with

Oops! Something went wrong! :(

ESLint: 9.20.1

ESLint couldn't find an eslint.config.(js|mjs|cjs) file.

It was last successful 11 months ago.

The workflow installs eslint with no version specified, therefore eslint@latest is used (currently [email protected]).

ESLint 9.0.0 changed the default config to flat.

How did you fix it?

Set ESLINT_USE_FLAT_CONFIG environment variable to false in the workflow .github/workflows/e2e-eslint-workflow.yml.

See Flat config is now the default and has some changes.

This will continue to work until ESLint 10 is released and support for deprecated .eslintrc config files is planned to be dropped. See eslintrc removed in ESLint v10.0.0.

Checklist

  • I have read the Contributing Guide.
  • I have set the packages that need to be released for my changes to be effective. (none)
  • I will check that all automated PR checks pass before the PR gets reviewed.

@MikeMcC399
Copy link
Contributor Author

Workflow https://github.com/yarnpkg/berry/actions/workflows/e2e-eslint-workflow.yml is now passing. 👍🏻

The Annotations are expected:

image

  1. The first two test failures are due to the workflow testing error conditions that ESLint should flag as linting errors.

  2. The cache errors relate to a known issue with GitHub Actions caching "Warning: Cache not found" on first run in branch actions/cache#1552 which is used by the workflow .github/actions/prepare/action.yml

@MikeMcC399 MikeMcC399 marked this pull request as ready for review February 16, 2025 09:33
@MikeMcC399 MikeMcC399 requested a review from arcanis as a code owner February 16, 2025 09:33
@arcanis
Copy link
Member

arcanis commented Feb 17, 2025

Can we rather update the test to use flat config?

@MikeMcC399
Copy link
Contributor Author

MikeMcC399 commented Feb 17, 2025

@arcanis

Can we rather update the test to use flat config?

This was a quick fix aligned with the rest of the repo. Now that you've updated to use ESLint 9 elsewhere, it would be cleaner to update the tests as well to be flat config. I'm not sure how clean that would be to add a flat config written in one line like the legacy config is done.

You are welcome to take this over and do it yourself or you can leave me to see if I can solve it myself.

@MikeMcC399
Copy link
Contributor Author

@arcanis

I will replace this PR with one which migrates the workflow to flat config.

In the meantime I'm relegating the PR to draft pending closure.

@MikeMcC399 MikeMcC399 marked this pull request as draft February 17, 2025 17:06
@arcanis
Copy link
Member

arcanis commented Feb 17, 2025

Appreciated, thanks Mike!

@MikeMcC399
Copy link
Contributor Author

@MikeMcC399 MikeMcC399 closed this Feb 18, 2025
@MikeMcC399 MikeMcC399 deleted the fix/e2e-eslint branch February 18, 2025 10:58
arcanis pushed a commit that referenced this pull request Feb 18, 2025
## What's the problem this PR addresses?

- supersedes #6690

The workflow
[.github/workflows/e2e-eslint-workflow.yml](https://github.com/yarnpkg/berry/blob/master/.github/workflows/e2e-eslint-workflow.yml)
fails with

```text
Oops! Something went wrong! :(

ESLint: 9.20.1

ESLint couldn't find an eslint.config.(js|mjs|cjs) file.
```

It was last successful 11 months ago on Apr 5, 2024 coinciding with the
release of [ESLint
9.0.0](https://eslint.org/blog/2024/04/eslint-v9.0.0-released/) which
changed the default config to flat.

The workflow installs `eslint` with no version specified, therefore
`eslint@latest` is used (currently `[email protected]`).

## How did you fix it?

In the workflow
[.github/workflows/e2e-eslint-workflow.yml](https://github.com/yarnpkg/berry/blob/master/.github/workflows/e2e-eslint-workflow.yml)

each `.estlintrc` is changed to `eslint.config.mjs`

### Validating ESLint

The ESLint rule [semi](https://eslint.org/docs/latest/rules/semi) was
[deprecated](https://eslint.org/blog/2023/10/deprecating-formatting-rules/)
in [ESLint
v8.53.0](https://eslint.org/blog/2023/11/eslint-v8.53.0-released/) and
moved instead to
[@stylistic/eslint-plugin-js](https://eslint.style/packages/js).

Following the [ESLint Stylistic Migration
Guide](https://eslint.style/guide/migration) and using the recommended
approach of one single plugin, additionally install
[@stylistic/eslint-plugin](https://www.npmjs.com/package/@stylistic/eslint-plugin).

Use the rule [@stylistic/semi](https://eslint.style/rules/default/semi).

### Running the TypeScript integration test

[typescript-eslint
v7](https://typescript-eslint.io/blog/announcing-typescript-eslint-v7/)
allows
[switching](https://typescript-eslint.io/blog/announcing-typescript-eslint-v7/#switching-to-typescript-eslint)
from
[@typescript-eslint/parser](https://www.npmjs.com/package/@typescript-eslint/parser)
and
[@typescript-eslint/eslint-plugin](https://www.npmjs.com/package/@typescript-eslint/eslint-plugin)
to [typescript-eslint](https://www.npmjs.com/package/typescript-eslint).

## Checklist

- [X] I have read the [Contributing
Guide](https://yarnpkg.com/advanced/contributing).
- [X] I have set the packages that need to be released for my changes to
be effective. (none)
- [x] I will check that all automated PR checks pass before the PR gets
reviewed.
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.

2 participants