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

Upgrade to latest abp2dnr #11

Merged
merged 6 commits into from
Jan 4, 2024
Merged

Conversation

seia-soto
Copy link
Member

Fixes #9

Summary

  • Upgrade from @ghostery/abp2dnr to @eyeo/abp2dnr
  • Patched @eyeo/abp2dnr not to use Node native binding
  • Update test cases
  • One test cannot be fixed (TrackerDB filters)

Patched @eyeo/abp2dnr not to use Node native bindings

New module uses Node native bindings that cannot be used with Bunjs. Unfortunately, the use of native module removed. ../build/Release/isRegexSupported refers to the file ../build/Release/isRegexSupported.node.

diff --git a/node_modules/@eyeo/abp2dnr/lib/converter.js b/node_modules/@eyeo/abp2dnr/lib/converter.js
index e284b0e..69e49c2 100644
--- a/node_modules/@eyeo/abp2dnr/lib/converter.js
+++ b/node_modules/@eyeo/abp2dnr/lib/converter.js
@@ -19,14 +19,12 @@
 
 "use strict";
 
-const {isRegexSupported} = require("../build/Release/isRegexSupported");
 const {createConverter} = require("@eyeo/webext-ad-filtering-solution/adblockpluscore/lib/dnr/index.js");
 const {FilterParsingError} = require("@eyeo/webext-ad-filtering-solution/adblockpluscore/lib/filters/index.js");
 const {Filter} = require("@eyeo/webext-ad-filtering-solution/adblockpluscore/lib/filterClasses.js");
 
 function regexCheck(r) {
-  let result = isRegexSupported(r);
-  return result.isSupported;
+  return false;
 }
 
 let converter = createConverter({isRegexSupported: regexCheck});

TrackerDB filters test cannot be fixed

TrackerDB filters test failed but it's not resolvable anymore. Before this changes, normalize function from test/unit/helpers.js normalized the output rule. However, it doesn't work anymore as abp2dnr throws empty array ([]) instead of partial rule ([truthy value]).

I suggest to skip testing rules leading to incomplete comparison.

- Upgrade from @ghostery/abp2dnr to @eyeo/abp2dnr
- Patched @eyeo/abp2dnr not to use Node native binding
- Update test cases

TrackerDB filters test failed but it's not resolvable anymore. Before `normalize` function from `test/unit/helpers.js` normalized the output rule. However, it doesn't work anymore as abp2dnr throws empty array ([]) instead of partial rule ([truthy value]).
@seia-soto seia-soto added the enhancement New feature or request label Jan 2, 2024
@seia-soto seia-soto requested a review from chrmod January 2, 2024 09:38
@seia-soto seia-soto self-assigned this Jan 2, 2024
@chrmod chrmod marked this pull request as draft January 3, 2024 11:59
patches/@eyeo+abp2dnr+1.0.0.patch Outdated Show resolved Hide resolved
test/unit/converters/abp.spec.js Outdated Show resolved Hide resolved
@chrmod
Copy link
Member

chrmod commented Jan 3, 2024

We should ensure that all TrackerDB tests pass. The point of this project is to ensure different converters behave the same.

@seia-soto seia-soto requested a review from chrmod January 4, 2024 07:33
@chrmod chrmod marked this pull request as ready for review January 4, 2024 08:57
@chrmod chrmod merged commit 9d5276e into ghostery:main Jan 4, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update ABP converter
2 participants