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

WebUI: migrate ESLint rules #20727

Merged
merged 1 commit into from May 3, 2024
Merged

WebUI: migrate ESLint rules #20727

merged 1 commit into from May 3, 2024

Conversation

Chocobo1
Copy link
Member

@Chocobo1 Chocobo1 added the WebUI WebUI-related issues/changes label Apr 19, 2024
@Chocobo1 Chocobo1 added this to the 5.0 milestone Apr 19, 2024
@Chocobo1 Chocobo1 requested a review from a team April 20, 2024 04:21
@xavier2k6
Copy link
Member

@Chocobo1 A few questions, if you don't mind.

What about using es2023 here? (I'm not sure if latest can be used via env like parse options etc. i.e. ecmaversion : latest)

I noticed the qBittorrent-website repo uses ecmaVersion: 2021, could this be migrated to ecmaversion : latest?
https://github.com/qbittorrent/qBittorrent-website/blob/master/eslint.config.mjs#L17


Can v8 be unpinned now or do we still need v9 to mature a bit?

"eslint": "^8",
"eslint-plugin-html": "^8",

@Chocobo1
Copy link
Member Author

Chocobo1 commented Apr 27, 2024

What about using es2023 here? (I'm not sure if latest can be used via env like parse options etc. i.e. ecmaversion : latest)

Our webui targets browsers released from a year ago so es2023 is too new.

I noticed the qBittorrent-website repo uses ecmaVersion: 2021, could this be migrated to ecmaversion : latest?

Specifying the latest spec would require website visitors use a very up-to-date browser and would (possibly) block non-trivial portion of viewers, I wouldn't recommend that.
Also note that this option has nothing to do with the produced code. It is only for CI checking and some sort of declaration about the official supported version.

Can v8 be unpinned now or do we still need v9 to mature a bit?

It seems v9.x is stable now. I plan to migrate over but the config file would also need migrated.
If someone helps approving this PR and #20728 perhaps it could be realized faster.

@xavier2k6
Copy link
Member

Our webui targets browsers released from a year ago so es2023 is too new.

👍

It is only for CI checking and some sort of declaration about the official supported version.

Ahh...I thought it was only just related to the CI.


I think it would be ok though to bump pinned dependency of @11ty/eleventy to 2.0.1 as it's latest/bugfix release

https://github.com/qbittorrent/qBittorrent-website/blob/1ebeec692c1f932e4636416332dfbf84c1f68902/package.json#L15-L16

xavier2k6
xavier2k6 previously approved these changes Apr 27, 2024
@Chocobo1
Copy link
Member Author

I think it would be ok though to bump pinned dependency of @11ty/eleventy to 2.0.1 as it's latest/bugfix release
https://github.com/qbittorrent/qBittorrent-website/blob/1ebeec692c1f932e4636416332dfbf84c1f68902/package.json#L15-L16

You can use this calculator to check which version will be picked: https://semver.npmjs.com/
Put @11ty/eleventy as the package name and put ^2.0.0 as the version. It will pick the latest version available which will be 2.0.1
Also check out the Use the caret (aka hat) symbol, ^ paragraph below.

IMO it would be better if it was specified 2.x (or 2, equivalent but shorter). It seems ^2.0.0 is equivalent to 2 so I wasn't in a hurry to change it. You can submit a PR if you want or I'll do it later.

@xavier2k6
Copy link
Member

@Chocobo1 Thanks for the explanation & link, the change is trivial - so will leave it to you if you want to change it or not, there's a v3 in development....(alpha)

@Chocobo1
Copy link
Member Author

PR updated, resolved merge conflicts.

@Chocobo1 Chocobo1 requested a review from xavier2k6 April 29, 2024 05:03
@Chocobo1 Chocobo1 merged commit 79eb7b8 into qbittorrent:master May 3, 2024
10 of 14 checks passed
@Chocobo1 Chocobo1 deleted the eslint branch May 3, 2024 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebUI WebUI-related issues/changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants