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

[Feat] Refactor repo with build tools #14

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gund
Copy link

@gund gund commented Oct 22, 2023

Closes #6
Closes #5

Following the conversation regarding the maintainability of this codebase, here are changes that address them all:

  • Reuse as much code as possible between different extensions (core logic is fully reused, only small "glue" code is left per each extension)
  • Introduce a build step to be able to reuse code (using rspack as a fast build tool for both service worker and UI builds)
  • Refactor UI into components to split the code and make it more maintainable in the long run
  • Use NX monorepo to manage code (extensions are built as apps and reusable code is in libs)
  • Add MIT license to the repo

There are many changes so I would advice you to carefully review at least the main logic files in libs/core.

I would also strongly suggest adding a CI (Continuous Integration) to be able to constantly build extensions here in Github Actions and maybe even add an auto-release workflow to publish extensions automatically to respective stores.
And maybe adding some unit tests at least for the core logic would be great too.

For the future improvements I would also suggest dropping jQuery from UI and replacing it with something more modern like Bootstrap or similar.
Also it is probably a good idea to move out all the classes for adblocking into a config which could be fetched remotely so the extension does not have to be updated to pickup new config.

Please have a look and let me know what do you think.
Cheers!

@0x48piraj
Copy link
Owner

That looks awesome!

I'll review the changes once the weekend is over.

Copy link
Owner

@0x48piraj 0x48piraj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, I'm sorry but I got busy and didn't have time to review all of the code, it looks good but yesterday I updated the extension hastily fixing YouTube's new patch to make it work again smoothly.

Can you resolve the conflicts?

@0x48piraj 0x48piraj force-pushed the master branch 3 times, most recently from d845199 to 9ce81c8 Compare October 31, 2023 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Not-very-maintainable™ codebase Prototype pollution
2 participants