Allow option update#1686
Open
davidjbradshaw wants to merge 6 commits into
Open
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds an “options update” flow to iframe-resizer: re-calling connectResizer() on an already-bound iframe now updates stored settings and sends an update message to the child, with wrapper integrations across major frameworks to trigger rebinds on option changes.
Changes:
- Add a core update path (
connectResizer→updateIframe) that merges options, validates child version, and dispatches anupdatemessage. - Add child-side handling for
updatemessages to apply changed settings without unnecessarily re-touching page styles/listeners. - Add framework wrapper rebind logic (React/Vue/Svelte/Solid/Angular/Web Component) plus unit/e2e coverage for the new behavior.
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config/solid.post-build.js | Copy Solid source dependencies needed for “solid” export consumers. |
| tsconfig.json | Adjust TS root config (noEmit) and include Svelte component sources. |
| packages/web-component/index.ts | Support option/attribute updates by rebuilding options and rebinding. |
| packages/vue/iframe-resizer.vue | Rebind on prop changes via watch() using a shared options builder. |
| packages/svelte/IframeResizer.svelte | Rebind on reactive prop changes while skipping the initial mount bind. |
| packages/solid/wire-options.ts | New helper to pick “wire payload” options with Solid reactivity tracking. |
| packages/solid/IframeResizer.tsx | Rebind on relevant prop changes using createEffect() and picked wire options. |
| packages/react/options-key.ts | New stable dependency key builder for React option-change detection. |
| packages/react/index.tsx | Rebind connectResizer when wire-relevant props change (update path). |
| packages/react/index.test.tsx | Add test validating React rebind behavior on option change. |
| packages/core/setup/update.ts | New parent-side update flow: merge options and send UPDATE to child. |
| packages/core/setup/update.test.ts | Unit tests for update flow and dispatch gating. |
| packages/core/index.ts | Route “already setup” connectResizer calls into the update flow. |
| packages/core/index.test.ts | Update test expectations for “already setup” to call update flow. |
| packages/core/checks/min-child-version.ts | New guard requiring child v6+ for option updates. |
| packages/core/checks/min-child-version.test.ts | Unit tests for child version parsing/requirements. |
| packages/common/consts.ts | Add UPDATE message constant. |
| packages/child/received/update.ts | New child handler applying updated settings and selectively applying side-effects. |
| packages/child/received/update.test.ts | Unit tests for child update handler behavior and side-effect gating. |
| packages/child/received/process-request.ts | Register update handler in child request router. |
| packages/child/page/links.units.test.ts | Update mocks to reflect in-page-links gating by settings. |
| packages/child/page/links.ts | Make in-page linking idempotent and respect runtime enable/disable. |
| packages/child/page/links.test.ts | Add coverage for findTarget no-op when inPageLinks disabled. |
| packages/child/page/links.branches.test.ts | Adjust branch test setup for inPageLinks gating. |
| packages/child/events/mouse.ts | Gate mouse event forwarding on runtime settings.mouseEvents. |
| packages/child/events/mouse.test.ts | Add coverage for mouse forwarding disabled after settings update. |
| packages/angular/directive.ts | Add ngOnChanges rebind path when options input changes. |
| e2e/tests/shared/parent-methods.js | Add e2e test ensuring rebind sends update without breaking iframe attachment. |
Comment on lines
+16
to
+27
| function mergeOptions(id: string, options: Record<string, any>): void { | ||
| settings[id] = { | ||
| ...settings[id], | ||
| ...checkOptions(id, options), | ||
| } | ||
|
|
||
| if (hasMouseEvents(options)) settings[id].mouseEvents = true | ||
| if (hasOwn(options, 'mode')) settings[id].mode = setMode(options) | ||
|
|
||
| setOffsetSize(id, options) | ||
| setTargetOrigin(id) | ||
| } |
Comment on lines
+13
to
+24
| const hasMouseEvents = (options: Record<string, any>): boolean => | ||
| hasOwn(options, 'onMouseEnter') || hasOwn(options, 'onMouseLeave') | ||
|
|
||
| function mergeOptions(id: string, options: Record<string, any>): void { | ||
| settings[id] = { | ||
| ...settings[id], | ||
| ...checkOptions(id, options), | ||
| } | ||
|
|
||
| if (hasMouseEvents(options)) settings[id].mouseEvents = true | ||
| if (hasOwn(options, 'mode')) settings[id].mode = setMode(options) | ||
|
|
Comment on lines
+73
to
+74
| const target = e.target as Element | null | ||
| const link = target?.closest?.('a[href^="#"]') |
Comment on lines
+163
to
+171
| attributeChangedCallback( | ||
| name: string, | ||
| oldValue: string | null, | ||
| newValue: string | null, | ||
| ): void { | ||
| if (oldValue === newValue) return | ||
| if (!RESIZER_ATTR_MAP[name]) return | ||
| this.rebind() | ||
| } |
Comment on lines
+67
to
+74
| // Re-bind when iframe-resizer-relevant props change. The first mount above | ||
| // already created the binding, so subsequent calls take the update path in | ||
| // core, which sends an UPDATE message to the child. | ||
| // | ||
| // optionsKey is the only meaningful dep — it's a stable serialization of the | ||
| // props the resizer cares about. We read latest `props` and `onBeforeClose` | ||
| // through refs so the effect doesn't re-fire on unrelated re-renders. | ||
| const optionsKey = buildOptionsKey(props) |
Comment on lines
+5
to
+15
| import settings from '../values/settings' | ||
|
|
||
| export default function moveToAnchor(id: string, anchor: string): void { | ||
| typeAssert(anchor, STRING, 'moveToAnchor(anchor) anchor') | ||
|
|
||
| if (settings[id]?.inPageLinks !== true) { | ||
| throw new Error( | ||
| 'moveToAnchor() requires the "inPageLinks" option to be set to true', | ||
| ) | ||
| } | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.