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

implement simple event emitter for FiltersEngine and sub-classes #251

Merged
merged 1 commit into from
Aug 9, 2019

Conversation

remusao
Copy link
Collaborator

@remusao remusao commented Aug 7, 2019

Following events are implemented:

  • request-blocked
  • request-redirected
  • request-whitelisted
  • csp-injected
  • script-injected
  • style-injected

Fixes #80
Fixes #247

@remusao remusao force-pushed the implement-event-logger branch from 27be7e1 to 30d727d Compare August 7, 2019 22:21
@char101
Copy link

char101 commented Aug 8, 2019

Could there be a chance that these event names will conflict with the event names used in the user application or from another library?

@remusao
Copy link
Collaborator Author

remusao commented Aug 8, 2019

Could there be a chance that these event names will conflict with the event names used in the user application or from another library?

This should not happen as the events are "local" to the blocker instance. For example you can have two instances emitting the same events and register two different callbacks to each:

const blocker1 = ElectronBlocker.parse('||domain1.com');
const blocker2 = ElectronBlocker.parse('||domain2.com');

blocker1.on('request-blocked', () => { console.log('blocker1!'); });
blocker2.on('request-blocked', () => { console.log('blocker2!'); });

Did I understand your question correctly?

@char101
Copy link

char101 commented Aug 8, 2019

Sorry since I didn't see any example code I thought the events was implemented via electron's ipcMain/ipcRenderer (since I was using adblocker-electron). If the events is local to the adblocker then of course there should be no problem.

@remusao
Copy link
Collaborator Author

remusao commented Aug 8, 2019

Indeed! I tried to implement this mechanism in such a way that all blockers can benefit from it: WebExtension, Electron, Puppeteer, etc. You can see a small example here: https://github.com/cliqz-oss/adblocker/blob/30d727d240ceca0e52f0c2089d8290bef8a1eff4/packages/adblocker-electron-example/index.ts#L70

* request-blocked
* request-redirected
* request-whitelisted
* csp-injected
* script-injected
* style-injected
@remusao remusao force-pushed the implement-event-logger branch from 30d727d to 8c63861 Compare August 9, 2019 12:16
@remusao remusao merged commit 34f05ef into ghostery:master Aug 9, 2019
@btzsoft
Copy link

btzsoft commented Aug 13, 2019

@remusao when it's planned to be a new release for this? thx.

@remusao
Copy link
Collaborator Author

remusao commented Aug 13, 2019

@btzsoft I was planning on releasing this with a few other changes that are in the pipeline but not related. But actually there is no strong reason to not release what is already on master (could be today), then release the rest in a few days when it's ready.

@btzsoft
Copy link

btzsoft commented Aug 13, 2019

@remusao this would be great. Thank you!

@remusao remusao mentioned this pull request Aug 13, 2019
@remusao
Copy link
Collaborator Author

remusao commented Aug 13, 2019

@btzsoft done. Let me know if you encounter any issue.

@btzsoft
Copy link

btzsoft commented Aug 14, 2019

@remusao I have an warning, is this ok?

WARNING in ./node_modules/@cliqz/adblocker/dist/es6/src/compression.js 25:34-41
[2] "export 'factory' was not found in 'tsmaz'
[2]  @ ./node_modules/@cliqz/adblocker/dist/es6/src/data-view.js
[2]  @ ./node_modules/@cliqz/adblocker/dist/es6/src/engine/engine.js
[2]  @ ./node_modules/@cliqz/adblocker/dist/es6/adblocker.js
[2]  @ ./app/modules/Browser/Adblock.js
....

@btzsoft
Copy link

btzsoft commented Aug 14, 2019

the adblocker throws when try to parse

@remusao
Copy link
Collaborator Author

remusao commented Aug 14, 2019

the adblocker throws when try to parse

@btzsoft weird, this error should only happen if the installed version of tsmaz is 1.3.0 but the released version of the adblocker depends on 1.2.2. Can you check in your node_modules which version of tsmaz is installed locally?

@btzsoft
Copy link

btzsoft commented Aug 15, 2019

@remusao yes was 1.3.0, changed to 1.2.0 and it's working now.

@remusao remusao deleted the implement-event-logger branch September 5, 2019 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: New Feature 🚀 Increment minor version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

adblock-electron: events on content blocking Implement event logger
3 participants