-
Notifications
You must be signed in to change notification settings - Fork 11
Support configuring urlPropsIgnore via appdef app settings #1602
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4f3e993
ba4848e
d58ad2a
ad9286e
ab001fd
b46f122
c801a89
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,9 +46,33 @@ const debouncedReplaceState = debounce((state, _, url) => window.history.replace | |
| * @since 0.14.9 - New optional ignoreProps argument | ||
| */ | ||
| export function updateUrl(state: IAppUrlState, extraState?: any, ignoreProps?: string[] | undefined): void { | ||
| const st: any = { ...extraState }; | ||
| const ignoreSet = new Set((ignoreProps ?? []).map(k => k.toLowerCase())); | ||
| const st: any = {}; | ||
|
|
||
| // Preserve existing URL params by default, excluding ignored keys. | ||
| // Keys are normalised to lowercase so that a later write of the same | ||
| // key from IAppUrlState (which is always lowercase) overwrites the | ||
| // existing entry rather than creating a duplicate with different casing. | ||
| const currentParams = parseUrlParameters(window.location.href); | ||
| for (const k in currentParams) { | ||
| if (ignoreSet.has(k.toLowerCase())) { | ||
| continue; | ||
| } | ||
| st[k.toLowerCase()] = currentParams[k]; | ||
| } | ||
|
|
||
|
Comment on lines
+49
to
+63
|
||
| // Apply extra state next, unless explicitly ignored. | ||
| if (extraState != null) { | ||
| for (const k in extraState) { | ||
| if (ignoreSet.has(k.toLowerCase())) { | ||
| continue; | ||
| } | ||
| st[k] = extraState[k]; | ||
| } | ||
| } | ||
|
|
||
| for (const k in state) { | ||
| if (ignoreProps && ignoreProps.indexOf(k) >= 0) | ||
| if (ignoreSet.has(k.toLowerCase())) | ||
| continue; | ||
| const val: any = (state as any)[k]; | ||
| switch (k) { | ||
|
|
@@ -72,7 +96,7 @@ export function updateUrl(state: IAppUrlState, extraState?: any, ignoreProps?: s | |
| break; | ||
| } | ||
| } | ||
| const url = appendParameters(window.location.href, st, true, false, ignoreProps != null); | ||
| const url = appendParameters(window.location.href, st, true, false, true); | ||
| //window.history.replaceState(st, "", url); | ||
| debouncedReplaceState(st, "", url); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import { describe, it, expect } from "vitest"; | ||
| import { getEffectiveUrlPropsIgnore } from "../../src/containers/app"; | ||
|
|
||
| describe("getEffectiveUrlPropsIgnore", () => { | ||
| it("returns undefined when both prop and settings value are undefined", () => { | ||
| expect(getEffectiveUrlPropsIgnore(undefined, undefined)).toBeUndefined(); | ||
| }); | ||
|
|
||
| it("returns the prop value when settings value is undefined", () => { | ||
| expect(getEffectiveUrlPropsIgnore(["x", "y"], undefined)).toEqual(["x", "y"]); | ||
| }); | ||
|
|
||
| it("returns parsed settings value when prop is undefined", () => { | ||
| expect(getEffectiveUrlPropsIgnore(undefined, "x,y")).toEqual(["x", "y"]); | ||
| }); | ||
|
|
||
| it("merges prop value and parsed settings value", () => { | ||
| expect(getEffectiveUrlPropsIgnore(["scale"], "x,y")).toEqual(["scale", "x", "y"]); | ||
| }); | ||
|
|
||
| it("trims whitespace from settings values", () => { | ||
| expect(getEffectiveUrlPropsIgnore(undefined, " x , y ")).toEqual(["x", "y"]); | ||
| }); | ||
|
|
||
| it("filters empty entries from settings value", () => { | ||
| expect(getEffectiveUrlPropsIgnore(undefined, "x,,y")).toEqual(["x", "y"]); | ||
| }); | ||
|
|
||
| it("returns prop when settings value results in empty list after filtering", () => { | ||
| expect(getEffectiveUrlPropsIgnore(["scale"], ",,")).toEqual(["scale"]); | ||
| }); | ||
|
|
||
| it("returns undefined when prop is undefined and settings value results in empty list", () => { | ||
| expect(getEffectiveUrlPropsIgnore(undefined, " ,, ")).toBeUndefined(); | ||
| }); | ||
|
|
||
| it("lowercases all entries from prop", () => { | ||
| expect(getEffectiveUrlPropsIgnore(["X", "Y"], undefined)).toEqual(["x", "y"]); | ||
| }); | ||
|
|
||
| it("lowercases all entries from settings value", () => { | ||
| expect(getEffectiveUrlPropsIgnore(undefined, "X,Y,Scale")).toEqual(["x", "y", "scale"]); | ||
| }); | ||
|
|
||
| it("deduplicates entries across prop and settings", () => { | ||
| expect(getEffectiveUrlPropsIgnore(["x", "scale"], "x,y")).toEqual(["x", "scale", "y"]); | ||
| }); | ||
|
|
||
| it("deduplicates case-insensitively (prop uppercase, settings lowercase)", () => { | ||
| expect(getEffectiveUrlPropsIgnore(["X"], "x,y")).toEqual(["x", "y"]); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| // Tests for URL state synchronization behavior when ignored URL properties are configured. | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; | ||
| import { updateUrl } from "../../src/containers/url-state"; | ||
|
|
||
| describe("updateUrl", () => { | ||
| beforeEach(() => { | ||
| vi.useFakeTimers(); | ||
| const params = new URLSearchParams({ | ||
| resource: "Library://Samples/Sheboygan/Layouts/SheboyganAsp.WebLayout", | ||
| foo: "bar", | ||
| x: "1" | ||
| }); | ||
| window.history.replaceState({}, "", `/?${params.toString()}`); | ||
| }); | ||
|
Comment on lines
+6
to
+14
|
||
|
|
||
| afterEach(() => { | ||
| vi.clearAllTimers(); | ||
| vi.useRealTimers(); | ||
| }); | ||
|
|
||
| it("preserves existing non-ignored params when ignore props are configured", () => { | ||
| updateUrl({ x: 2, y: 3, scale: 1000 }, undefined, ["x", "y"]); | ||
| vi.runAllTimers(); | ||
|
|
||
| const params = new URL(window.location.href).searchParams; | ||
| expect(params.get("resource")).toBe("Library://Samples/Sheboygan/Layouts/SheboyganAsp.WebLayout"); | ||
| expect(params.get("foo")).toBe("bar"); | ||
| expect(params.get("x")).toBeNull(); | ||
| expect(params.get("y")).toBeNull(); | ||
| expect(params.get("scale")).toBe("1000"); | ||
| }); | ||
|
|
||
| it("leaves existing params alone when no ignore props are configured", () => { | ||
| updateUrl({ x: 2, y: 3, scale: 1000 }); | ||
| vi.runAllTimers(); | ||
|
|
||
| const params = new URL(window.location.href).searchParams; | ||
| expect(params.get("resource")).toBe("Library://Samples/Sheboygan/Layouts/SheboyganAsp.WebLayout"); | ||
| expect(params.get("foo")).toBe("bar"); | ||
| expect(params.get("x")).toBe("2"); | ||
| expect(params.get("y")).toBe("3"); | ||
| expect(params.get("scale")).toBe("1000"); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The URL sync effect compares
curUrlStatevsnextUrlStatewithareStatesEqual()before callingupdateUrl(), butareStatesEqual()does not account for ignored URL props. IfurlPropsIgnoreincludes keys thatnextUrlStatestill sets (egx/y/scale), the states will never be equal and this effect will callupdateUrl()on every run even when the effective URL cannot change. Consider stripping ignored keys fromnextUrlState(and/or readingcurUrlStatewith the same ignore list) before comparing to avoid redundantreplaceStatecalls.