MWPW-135873 Compress the caas hash strings #1401
Conversation
By compressing using gzip, the hash length is reduced by ~60% The new compressed hashes start with `~~` in order to differentiate them from the original non-compressed format.
|
Hello, I'm the AEM Code Sync Bot and I will run some test suites that validate the page speed.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
chrischrischris
added
the
needs-verification
|
|
| @@ -36,13 +43,15 @@ const updateObj = (obj, defaultObj) => { | |||
|
|
|||
| const isValidUuid = (id) => /^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$/.test(id); | |||
|
|
|||
| const getHashConfig = () => { | |||
| const getHashConfig = async () => { | |||
There was a problem hiding this comment.
I don't see an await in this function, async needed?
There was a problem hiding this comment.
getHashConfig now can either be async (for v2 hashes) or sync (for v1 hashes). I think for clarity, since it's possible that it will be async this function should be marked as async? Otherwise you have to just know. Thoughts?
There was a problem hiding this comment.
Nevermind, am updating the code to show why it's async.
| useEffect(() => { | ||
| const setInitialState = async () => { | ||
| const initialState = await getInitialState(); | ||
| dispatch({ type: 'SET_STATE', value: initialState }); | ||
| }; | ||
|
|
||
| if (!Object.keys(state).length) { | ||
| setInitialState(); | ||
| } | ||
| }, [state]); | ||
|
|
There was a problem hiding this comment.
Could you simplify this by putting an empty [] instead of [state] so the useEffect runs only once? Might be able to omit the check for state length on line 955 if so.
|
|
| const a = document.getElementById('v2-caaslink'); | ||
| await init(a); | ||
| await waitForElement('#caas'); | ||
| await delay(5); |
There was a problem hiding this comment.
Shoul we use a fake timer instead? I'm fine with the delay as well if it's deterministic such as waiting for an eventListener execution and doesn't lead to flakiness.
|
rgclayton
added
verified
|
|
chrischrischris
force-pushed
the
caaslink-135873
branch
from
83cae41 to
73fe332
Compare
|
chrischrischris
force-pushed
the
caaslink-135873
branch
from
73fe332 to
be01bd7
Compare
|
|
Codecov Report
@@ Coverage Diff @@
## main #1401 +/- ##
=======================================
Coverage 96.32% 96.32%
=======================================
Files 144 144
Lines 34414 34513 +99
=======================================
+ Hits 33148 33244 +96
- Misses 1266 1269 +3
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
This reverts commit 202c33f.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Sunil Kamat <107644736+sukamat@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Sunil Kamat <107644736+sukamat@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Sunil Kamat <107644736+sukamat@users.noreply.github.com>
The hash string is now compressed using gzip: the hash length is reduced by ~60%
The new compressed hashes start with
~~in order to differentiate them from the original non-compressed format.The configurator at /tools/caas will now only generate the v2 hashes, but pages can use links with either format.
Resolves: MWPW-135873
Test URLs: