-
Notifications
You must be signed in to change notification settings - Fork 63
Update compiler to v3.1.3 and agtree to v3.2.4 #1118
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other comments (2)
- .github/workflows/build-3p.yaml (33-33) This PR updates Node.js from 20.x to 22.x. Since Node.js 22.x is still relatively new, have you verified that all dependencies (especially the updated `@adguard/filters-compiler` v3.0.2 and other packages) are fully compatible with this version? If not, it might be worth testing thoroughly to ensure there are no compatibility issues.
-
.eslintignore (1-3)
The file is missing a newline at the end. It's a good practice to end text files with a newline character.
platforms/ scripts/build/ scripts/validation/
1 file skipped due to size limits:
- yarn.lock
💡 To request another review, post a new comment with "/windsurf-review".
| }, | ||
| "engines": { | ||
| "node": ">=18" | ||
| "node": ">=22" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which part of the code requires at least node 22?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adguard/[email protected] expected version ">=22"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think its too strict, probably we can reduce it in agtree as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer not to decrease it in agtree,
especially when node.js v18 is no longer maintained now:
https://nodejs.org/en/about/previous-releases
let's keep it 22
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generally looks good to me, nice work. just left some comments, please check them
|
NodeJS 24 will be entered into its active LTS period in 2025-10-28. |
we may keep 22 until it is maintained |
Update
@adguard/filters-compilertov3.1.3and@adguard/agtreetov3.2.4Dependencies' changes inside
@adguard/filters-compilerv3.1.3Fixes included
remove:true;rules to AdGuard syntax FiltersCompiler#236[$domain]cosmetic filters existing in a 3rd-party filters list are missing FiltersCompiler#258