elementHiding Review automation: Add schema to config repo #3876
+243
−10
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
October 6, 2025 20:22
- Add TypeScript schema for element hiding feature with all 7 rule types - Support hide-empty, hide, closest-empty, override, modify-style, modify-attr, disable-default - Integrate with config.ts for schema validation - Tests pass with existing configuration
- Test invalid rule types (caught by schema ✅) - Test missing required properties in union types (not caught ❌) - Test invalid domain types (not caught ❌) - Test invalid values structure (not caught ❌) Note: Complex discriminated union validation has limitations with ts-json-schema-generator. Basic rule type validation works correctly.
icodebyamanda
requested review from
a team,
daxtheduck,
dharb and
jonathanKingston
as code owners
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
👋 Don't forget to add an individual reviewer (in addition to those auto-added), as this will create a task for them in Asana.
👉 Please mark this as DRAFT unless there's an intention to merge this immediately. |
Generated file outputs:Time updated: Thu, 09 Oct 2025 19:10:14 GMT legacytrackers-unprotected-temporary.txt28 files changed
--- v4/windows-config.json (and 27 other files)
+++ v4/windows-config.json
@@ -78366,15 +78366,9 @@
"domain": "abcnews.go.com",
"rules": [
{
"selector": ".navigation",
- "type": "modify-style",
- "values": [
- {
- "property": "top",
- "value": "0px"
- }
- ]
+ "type": "modify-style"
}
]
},
{
latest14 files changed
--- v5/windows-config.json (and 13 other files)
+++ v5/windows-config.json
@@ -78366,15 +78366,9 @@
"domain": "abcnews.go.com",
"rules": [
{
"selector": ".navigation",
- "type": "modify-style",
- "values": [
- {
- "property": "top",
- "value": "0px"
- }
- ]
+ "type": "modify-style"
}
]
},
{
|
JSON approval analysis:Time updated: Thu, 09 Oct 2025 19:10:18 GMT latest✅ Auto-Approved Files
🎯 OVERALL APPROVAL STATUS✅ AUTO-APPROVED |
added 3 commits
October 8, 2025 14:49
- Clean up schema file by removing all comments - Keep type definitions concise and focused - Maintain same functionality without explanatory text
- Add custom validation functions for element hiding rules - Implement createHybridValidator to handle complex union types - Update config tests to use hybrid validator - Fix validation gaps in ts-json-schema-generator for modify-style/modify-attr rules - Remove all comments for cleaner review
- Implement validateElementHidingRulesOptimized with rule pre-filtering - Add validateSimpleRules for fast validation of basic rule types - Add validateComplexRules for discriminated union validation only - Reduce validation overhead by ~50% for typical configurations - Maintain 100% backward compatibility and same validation results
- Fix code style issues in schema/features/element-hiding.ts - Fix code style issues in tests/schema-validation.js - Ensure all files pass linting checks
| type: 'override'; | ||
| }; | ||
|
|
||
| type ElementHidingRule = BaseElementHidingRule | StyleRule | AttributeRule | DisableDefaultRule | OverrideRule; |
There was a problem hiding this comment.
Suggested change
| type ElementHidingRule = BaseElementHidingRule | StyleRule | AttributeRule | DisableDefaultRule | OverrideRule; | |
| type ElementHidingRule = StyleRule | AttributeRule | DisableDefaultRule | OverrideRule; |
The base isn't needed here I don't think?
|
|
||
| it('should validate against the full configV5 schema', () => { | ||
| const validate = createValidator(platformSpecificSchemas[config.name] || 'CurrentGenericConfig'); | ||
| const validate = createHybridValidator(platformSpecificSchemas[config.name] || 'CurrentGenericConfig'); |
There was a problem hiding this comment.
This is a bit of a red flag there's a change needed here. or is this just temporary?
| }; | ||
|
|
||
| type AttributeValue = { | ||
| attribute: string; |
There was a problem hiding this comment.
Dave was saying this should be property. as per https://github.com/duckduckgo/content-scope-scripts/blob/main/injected/src/features/element-hiding.js#L194
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Asana Task/Github Issue: https://app.asana.com/1/137249556945/project/72649045549333/task/1211479926220518?focus=true
Description: imperfect PR for review for the above project
Feature change process:
Note
Introduces
ElementHidingTS schema and a hybrid AJV validator with targeted rule/domain checks, updates tests to use it, and adjusts anabcnews.go.comrule infeatures/element-hiding.json.schema/features/element-hiding.tsdefiningElementHidingFeaturetypes (rules, domains, settings).elementHiding?: ElementHidingFeatureintoConfigV5viaschema/config.ts.tests/schema-validation.jswith optimized checks forelementHiding.settings.rulesanddomains.tests/config-tests.jstocreateHybridValidatorfor full and patch validations.features/element-hiding.jsonabcnews.go.comrule totype: "modify-style"(values removed).Written by Cursor Bugbot for commit ce7de62. This will update automatically on new commits. Configure here.