Clarify filters and versions for processing#291
Clarify filters and versions for processing#291aselbie wants to merge 1 commit intofacebookincubator:mainfrom
Conversation
|
Hi @aselbie! Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours needs attention. You currently have a record in our system, but the CLA is no longer valid, and will need to be resubmitted. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
This PR fixes some edge case issues with version and filterType, and updates the code to make it more obvious how they interact to control processing of scripts.
Changes
Address
currentFilterTypeedge caseChanges the currently global
currentFilterTypetofilterTypeByVersionwhich is a map of filter type per version.This hardens a potential fragile point of the code:
currentFilterTypeto BOTH.Currently, this most likely cannot happen since our server side code injects new manifests into the page at the same time, however this should protect us from any future changes, both on the client and server side.
Assume script belong to latest version
In cases where we don't know what version a script belongs to, we now push to the latest version in FOUND_SCRIPTS rather than the first. For the initial page load where we are pushing scripts into the UNKNOWN version (for WhatsApp) this is unlikely to matter, but for scripts encountered later this should avoid false negatives.
More granular filter types
Until we started revisiting this code a few versions ago, the "empty string" filter type was doing a lot of heavy lifting. I've added several specific filter types:
filterTypeByVersion.Don't mutate scripts when transferring them to FOUND_SCRIPTS
For WhatsApp, we don't know the versions of scripts we encounter until we've processed the manifest so we store them in an unknown version queue. When we process the manifest we move those scripts with unknown versions into the queue of scripts for that manifest's version.
Previously when we did this we would mutate the ScriptDetails objects so that they would also have the filterType for that manifest. Rather than do this mutation, the new MANIFEST_LOADED filter will mean these scripts will always just wait until a manifest is loaded to start processing. This is probably cleaner in either chase, but was necessary because
currentFilterTypeis no longer available globally.Clarifications
otherTypetofilterTypeRequiredToProcesseverywhere to be more descriptive of how it is used.Testing