Skip to content
This repository has been archived by the owner on May 3, 2024. It is now read-only.

fix(reactHtml): renderScripts adding unknown integrity #1166

Merged
merged 7 commits into from
Nov 13, 2023

Conversation

JAdshead
Copy link
Contributor

@JAdshead JAdshead commented Oct 31, 2023

Description

Fallback externals do not support legacy, remove legacy logic to ensure does not load missing scripts.
Warn when missing integrity when rendering scripts

Motivation and Context

Fix SSR when fallback externals are required and has no integrity value provided

How Has This Been Tested?

Test suite

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (adding or updating documentation)
  • Dependency update
  • Security update

Checklist:

  • My change requires a change to the documentation and I have updated the documentation accordingly.
  • These changes should be applied to a maintenance branch.
  • This change requires cross browser checks.
  • Performance tests should be ran against the server prior to merging.
  • This change impacts caching for client browsers.
  • This change impacts HTTP headers.
  • This change adds additional environment variable requirements for One App users.
  • I have added the Apache 2.0 license header to any new files created.

What is the Impact to Developers Using One App?

Copy link
Contributor

github-actions bot commented Oct 31, 2023

Size Change: +215 B (0%)

Total Size: 712 kB

Filename Size Change
./build/app/app~vendors.js 411 kB +215 B (0%)
ℹ️ View Unchanged
Filename Size
./build/app/app.js 164 kB
./build/app/runtime.js 7.07 kB
./build/app/service-worker-client.js 7.25 kB
./build/app/vendors.js 123 kB

compressed-size-action

@Matthew-Mallimo
Copy link
Member

Matthew-Mallimo commented Nov 1, 2023

fallback externals do not include integrity values

But shouldn't they?

@JAdshead
Copy link
Contributor Author

JAdshead commented Nov 6, 2023

fallback externals do not include integrity values

But shouldn't they?

Yes, but instead of crashing in the client this will more gracefully load the script

@JAdshead JAdshead force-pushed the fix/render-scripts-integrity branch from e6e3b23 to e714df7 Compare November 6, 2023 16:45
@JAdshead JAdshead force-pushed the fix/render-scripts-integrity branch from e714df7 to 7c5e83e Compare November 6, 2023 16:46
package.json Show resolved Hide resolved
dogpatch626
dogpatch626 previously approved these changes Nov 8, 2023
@@ -112,7 +112,7 @@
"fastify-plugin": "^4.5.1",
"flat": "^5.0.2",
"helmet": "^7.0.0",
"holocron": "^1.8.2",
"holocron": "^1.9.1",
Copy link
Member

Choose a reason for hiding this comment

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

I this required? If I remember correctly the 1.9.0 release was done unintentionally?

Copy link
Contributor Author

@JAdshead JAdshead Nov 13, 2023

Choose a reason for hiding this comment

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

yes, it includes the fixes for integrity loading in holocron - americanexpress/holocron#162

@JAdshead JAdshead force-pushed the fix/render-scripts-integrity branch from e1379d2 to 942d8a8 Compare November 13, 2023 18:20
@JAdshead JAdshead merged commit 6ae1e65 into main Nov 13, 2023
7 checks passed
@JAdshead JAdshead deleted the fix/render-scripts-integrity branch November 13, 2023 18:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants