Skip to content

chore: bump ESLint to v9 #371

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 33 commits into from
Apr 9, 2025
Merged

chore: bump ESLint to v9 #371

merged 33 commits into from
Apr 9, 2025

Conversation

barjin
Copy link
Contributor

@barjin barjin commented Mar 26, 2025

Updates the code linting tooling to the latest versions and fixes the newly arisen issues.

Closes #338

@barjin barjin added the adhoc Ad-hoc unplanned task added during the sprint. label Mar 26, 2025
@barjin barjin self-assigned this Mar 26, 2025
@github-actions github-actions bot added this to the 111th sprint - Tooling team milestone Mar 26, 2025
@github-actions github-actions bot added the t-tooling Issues with this label are in the ownership of the tooling team. label Mar 26, 2025
@barjin barjin marked this pull request as draft March 26, 2025 14:36
@github-actions github-actions bot added the tested Temporary label used only programatically for some analytics. label Mar 26, 2025
@barjin barjin marked this pull request as ready for review March 27, 2025 13:15
@barjin barjin force-pushed the chore/eslint-v9-bump branch from 3c9c7ee to 9df1b92 Compare March 27, 2025 13:43
@barjin barjin force-pushed the chore/eslint-v9-bump branch from 9df1b92 to 9513ca3 Compare March 27, 2025 13:52
barjin
barjin commented Mar 27, 2025
View reviewed changes
test/e2e/scrapers/cheerio-ignoreSslErrors-disabled/test.mjs Outdated
@@ -24,6 +24,7 @@ await run(testDir, 'cheerio-scraper', {
switch (label) {
case 'START': return handleStart(context);
case 'DETAIL': return handleDetail(context);
default: throw new Error(`Unrecognized request label: ${label}`);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Throwing an exception might be too harsh, but we won't get here anyway, right? Alternatively, we could log the same message with log.error

@barjin barjin requested review from Copilot and janbuchar and removed request for Copilot March 27, 2025 15:57
B4nan
B4nan requested changes Apr 1, 2025
View reviewed changes
Copy link
Member

@B4nan B4nan left a comment

Choose a reason for hiding this comment

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

i'd vote for adding prettier/biome right ahead to fix the wrong formatting of the changed imports

janbuchar
janbuchar requested changes Apr 7, 2025
View reviewed changes
Copy link
Contributor

@janbuchar janbuchar left a comment

Choose a reason for hiding this comment

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

Please add biome while you're at it 🙂

@janbuchar janbuchar self-requested a review April 8, 2025 08:15
janbuchar
janbuchar approved these changes Apr 8, 2025
View reviewed changes
Copy link
Contributor

@janbuchar janbuchar left a comment

Choose a reason for hiding this comment

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

LGTM if you manage to fix the tests 🙂 Good stuff!

@barjin barjin force-pushed the chore/eslint-v9-bump branch from b9bf747 to 253de12 Compare April 8, 2025 10:23
@barjin
Copy link
Contributor Author

barjin commented Apr 8, 2025

I noticed we're still keeping the .eslintrc.json files in the test/e2e and website/ folders.

We'll probably want to switch this before merging this PR, right?

@B4nan
Copy link
Member

B4nan commented Apr 8, 2025

Yeah, old configs should be removed, eslint 9 won't read them anyway.

B4nan
B4nan approved these changes Apr 8, 2025
View reviewed changes
Copy link
Member

@B4nan B4nan left a comment

Choose a reason for hiding this comment

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

too huge to be reviewed 🙃

@barjin
Copy link
Contributor Author

barjin commented Apr 9, 2025

With the last commit, I consolidated the eslint setup for the actual source files and the website / docs examples. This uncovered some issues (which I fixed), and now I have this last one here:

image

image

Was apify/src ever a thing, or is it rather a remnant of local development (that somehow made it to the docs)?

@B4nan
Copy link
Member

B4nan commented Apr 9, 2025

I think apify/src is just a bad import produced by some IDE, I know webstorm likes to do this in monorepos, so could be from me actually 🙃 .

@barjin
Copy link
Contributor Author

barjin commented Apr 9, 2025 •
edited
Loading

Alright, I'll change it to 'apify' (since that's what's in all the other docs versions of this file) and we're done ( 🤞 )

@barjin barjin merged commit bb6afcd into master Apr 9, 2025
8 checks passed
@barjin barjin deleted the chore/eslint-v9-bump branch April 9, 2025 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@janbuchar janbuchar janbuchar approved these changes

@B4nan B4nan B4nan approved these changes

Assignees

@barjin barjin

Labels
adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.
Projects
None yet
Milestone
111th sprint - Tooling team
Development

Successfully merging this pull request may close these issues.

Update eslint to v9, use new flat Apify eslint config
3 participants
@barjin @B4nan @janbuchar