Skip to content

Attach allowed origins changed also to hook in useSaveStateHook#1415

Merged
abcampo-iry merged 1 commit intomainfrom
issues/1225-also-affects-save-state
Apr 1, 2026
Merged

Attach allowed origins changed also to hook in useSaveStateHook#1415
abcampo-iry merged 1 commit intomainfrom
issues/1225-also-affects-save-state

Conversation

@abcampo-iry
Copy link
Copy Markdown
Contributor

@abcampo-iry abcampo-iry commented Mar 31, 2026

@abcampo-iry abcampo-iry temporarily deployed to previews/1415/merge March 31, 2026 20:33 — with GitHub Actions Inactive
@abcampo-iry abcampo-iry changed the title 1255/1274 - attach allowed origins changed also to hook in useSaveStateHook Attach allowed origins changed also to hook in useSaveStateHook Mar 31, 2026
@abcampo-iry abcampo-iry marked this pull request as ready for review March 31, 2026 20:35
Copilot AI review requested due to automatic review settings March 31, 2026 20:35
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request updates the useScratchSaveState hook to use the getScratchAllowedOrigin() utility function instead of directly using process.env.ASSETS_URL, aligning with the fix introduced in PR #1413. This ensures proper origin validation for postMessage communication with the Scratch iframe when ASSETS_URL includes a path component.

Changes:

  • Updated useScratchSaveState.js to import and use getScratchAllowedOrigin() for origin validation instead of inline origin logic
  • Updated useScratchSaveState.test.js to properly mock getScratchAllowedOrigin and added test coverage for the edge case where ASSETS_URL contains a path
  • The hook now correctly normalizes asset URLs to their origin for postMessage validation, matching the implementation pattern used in ScratchContainer.jsx and other scratch-related utilities

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/hooks/useScratchSaveState.js Updated to call getScratchAllowedOrigin() for origin validation instead of directly using process.env.ASSETS_URL
src/hooks/useScratchSaveState.test.js Added import and mock for getScratchAllowedOrigin, updated test setup, and added new test case for ASSETS_URL with path component

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

@zetter-rpf zetter-rpf left a comment

Choose a reason for hiding this comment

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

Nice spot

@abcampo-iry abcampo-iry merged commit 3aa72aa into main Apr 1, 2026
7 checks passed
@abcampo-iry abcampo-iry deleted the issues/1225-also-affects-save-state branch April 1, 2026 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants