fix(@angular-devkit/build-angular): generate different content hashes for scripts which are changed during the optimization phase by martinfrancois · Pull Request #23531 · angular/angular-cli

martinfrancois · 2022-07-08T18:04:20Z

Instead of generating the content hash based on the content of scripts BEFORE the optimization phase,
the content hash is generated AFTER the optimization phase.

Prevents caching issues where browsers block execution of scripts due to the integrity hash not matching
with the cached script in case of a script being optimized differently than in a previous build,
where it would previously result in the same content hash.

Fixes #22906

Test case is based on the reproduction example as specified in #22906 (thanks!).

packages/angular_devkit/build_angular/src/webpack/plugins/scripts-webpack-plugin.ts

tests/legacy-cli/e2e/tests/build/output-hashing.ts

alan-agius4

Thanks for your contribution.

Couple of small changes.

martinfrancois · 2022-07-11T12:16:51Z

Thanks for your contribution.

Couple of small changes.

@alan-agius4 You're welcome, thanks a lot for the quick and thorough review! I agree with all of your suggestions and addressed all of your comments and pushed the changes, can you please have a look again?

tests/legacy-cli/e2e/tests/build/scripts-output-hashing.ts

alan-agius4

LGTM. one small NIT

Thanks for this.

martinfrancois · 2022-07-11T12:36:45Z

@alan-agius4 You're welcome, thanks as well for the quick handling! Addressed your nit now and pushed it.

alan-agius4 · 2022-07-11T14:07:02Z

@martinfrancois, it looks like

angular-cli/packages/angular_devkit/build_angular/src/builders/browser/specs/scripts-array_spec.ts

Lines 78 to 101 in 68358f0

    
           it('uglifies, uses sourcemaps, and adds hashes', async () => { 
        
             host.writeMultipleFiles(scripts); 
        
             const { files } = await browserBuild(architect, host, target, { 
        
               optimization: true, 
        
               sourceMap: true, 
        
               outputHashing: 'all', 
        
               scripts: getScriptsOption(), 
        
             } as {}); 
        
             const fileNames = Object.keys(files); 
        
             const scriptsBundle = fileNames.find((n) => /scripts\.[0-9a-f]{16}\.js/.test(n)); 
        
             expect(scriptsBundle).toBeTruthy(); 
        
             expect(await files[scriptsBundle || '']).toMatch('var number=2;'); 
        
             expect(fileNames.some((n) => /scripts\.[0-9a-f]{16}\.js\.map/.test(n))).toBeTruthy(); 
        
             expect(fileNames.some((n) => /renamed-script\.[0-9a-f]{16}\.js/.test(n))).toBeTruthy(); 
        
             expect(fileNames.some((n) => /renamed-script\.[0-9a-f]{16}\.js\.map/.test(n))).toBeTruthy(); 
        
             expect(fileNames.some((n) => /script\.[0-9a-f]{16}\.js/.test(n))).toBeTruthy(); 
        
             expect(await files['lazy-script.js']).not.toBeUndefined(); 
        
             expect(await files['lazy-script.js.map']).not.toBeUndefined(); 
        
             expect(await files['renamed-lazy-script.js']).not.toBeUndefined(); 
        
             expect(await files['renamed-lazy-script.js.map']).not.toBeUndefined(); 
        
           });

is failing.

Let me know if you need any help with this.

alan-agius4 · 2022-07-11T14:16:02Z

It looks like the sourcemaps are not renamed. In this case there is also another problem.

This also highlights a bigger problem. Since the sourcemaps are being renamed afterwards the sourcemaps comment would point to an invalid sourcemap file.

I think one way to fix this would be to change the hook to maybe PROCESS_ASSETS_STAGE_DEV_TOOLING or something after PROCESS_ASSETS_STAGE_OPTIMIZE_SIZE.

https://webpack.js.org/api/compilation-hooks/

martinfrancois · 2022-07-11T14:31:00Z

@alan-agius4 I just noticed it as well and came to the same conclusion as you, I tried to run the tests locally but so far I was not able to, when I run them using yarn bazel:test I'm getting the following error (I'm running on macOS, not sure if that should be a problem) and wasn't able to find a solution to it online. Is there a different way of running the tests without Bazel or do you happen to know how to fix the error?

alan-agius4 · 2022-07-11T14:51:46Z

I do recall @clydin had problems running on Mac M1.

Bazel is to only way to run these tests.

martinfrancois · 2022-07-11T14:54:27Z

@alan-agius4 I'm not using an M1 Mac, just an Intel one, that's the strange part about it... Is there a way to restrict the tests to only test scripts-array_spec.ts or the package it is in, without running the whole test suite?

… for scripts which are changed during the optimization phase Instead of generating the content hash based on the content of scripts BEFORE the optimization phase, the content hash is generated AFTER the optimization phase. Prevents caching issues where browsers block execution of scripts due to the integrity hash not matching with the cached script in case of a script being optimized differently than in a previous build, where it would previously result in the same content hash. Fixes angular#22906

martinfrancois · 2022-07-12T08:00:45Z

@alan-agius4 Your intuition seems to have been correct, changing the hook to PROCESS_ASSETS_STAGE_DEV_TOOLING did the trick, can you have a look again please?
By the way, I was not able to get the tests working on Mac and also not on Windows, they ended up working nicely in Ubuntu 22.04 though.

martinfrancois · 2022-07-12T08:24:58Z

@alan-agius4 Tests are passing now! 🎉

alan-agius4

LGTM

olegomon · 2022-07-12T10:39:29Z

@martinfrancois thanks for the fix. @alan-agius4 thanks for your quick response on this issue.

angular-automatic-lock-bot · 2022-08-12T08:03:21Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}