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.

Sign up for GitHub

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

Jump to bottom

Move last legacy unit-test to esbuild #2140

Merged
merged 8 commits into from
Jul 26, 2023
Merged

Move last legacy unit-test to esbuild #2140

merged 8 commits into from
Jul 26, 2023

Conversation

sammacbeth
Copy link
Collaborator

Description:

  • The declarative-net-request unit-test was the last test stuck on browserify.
  • ESbuild prevented us from stubbing the startup.ready function, which is used by our DNR code to wait for certain lists to be loaded.
  • We can refactor that code to wait specifically for the surrogates list to be loaded, and remove the startup dependency.

@sammacbeth
Move DNR unit-test off browserify
d888cc2
@sammacbeth
Check for surrogates loaded directly in dnr-config-rulesets.
94954a1 
We were using startup.ready as a proxy for surrogates being loaded, but we can check that explicitly from tdsStorage.
@sammacbeth
Fix lint
98fc31d
@sammacbeth
Fix UA parsing in node testing
896b56b
@sammacbeth
Remove browserify
d817849
@sammacbeth
Wait for all tds stores to be ready before generating config rulesets.
75eee71 
Turns out we can't just wait for surrogates, because on startup we load
all lists in parallel, then only update the trackers class once they've
all loaded.
@sammacbeth sammacbeth marked this pull request as ready for review July 25, 2023 19:10
@sammacbeth sammacbeth requested a review from kzar July 25, 2023 19:10
kzar
kzar reviewed Jul 26, 2023
View reviewed changes
Makefile Show resolved Hide resolved
@kzar
Copy link
Collaborator

kzar commented Jul 26, 2023

I also notice there are references to Browserify in packages/privacy-grade. Can those (and the corresponding Gruntfile) be removed? If not, would removing Browserify as a dependency here break the privacy-grade package somehow?

kzar
kzar reviewed Jul 26, 2023
View reviewed changes
package.json Show resolved Hide resolved
@kzar
Copy link
Collaborator

kzar commented Jul 26, 2023

I also think scripts/rewrite-meta.js can be removed?

@sammacbeth
Copy link
Collaborator Author

Thanks @kzar. Regarding privacy-grade, that module transitively depends on browserify via grunt-browserify, so it won't get broken by this change. I'll make a follow-up PR to update the build system for that module.

@sammacbeth sammacbeth merged commit 3cb58ff into duckduckgo:main Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kzar kzar kzar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sammacbeth @kzar