Skip to content

Refactor init command to be a dispatchable init action#1643

Draft
Copilot wants to merge 9 commits into
masterfrom
copilot/refactor-init-command-to-dispatchable-action
Draft

Refactor init command to be a dispatchable init action#1643
Copilot wants to merge 9 commits into
masterfrom
copilot/refactor-init-command-to-dispatchable-action

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 5, 2026

The viewer initialization flow required callers to construct an IViewerInitCommand object that both fetches the appdef resource and builds the INIT_APP payload — there was no way to initialize directly from an already-loaded ApplicationDefinition.

Changes

IViewerInitCommand interface (src/actions/init-command.ts)

  • Added required runFromAppDefAsync(appDef, options) method that builds the full IInitAppActionPayload from a pre-loaded ApplicationDefinition, handling locale init, session setup/reuse, runtime-map creation, toolbar parsing, and selection-set recovery.
  • loadResourceAsync is not on this interface — resource loading is fully decoupled from the interface and from any Redux dispatch mechanism.

IInitAsyncOptions (src/actions/init.ts)

  • Added optional sessionWasReused? flag for threading session context into runFromAppDefAsync, distinguishing a freshly-created session from a genuinely reused one.

DefaultViewerInitCommand (src/actions/init-mapguide.ts)

  • Implemented loadResourceAsync as a concrete public method — handles all resource-id cases (appdef.json, URL-to-appdef, MapGuide ApplicationDefinition, WebLayout, function-resourceId), creates a session lazily when required, and returns a LoadedResource discriminated union ({ kind: 'appdef', appDef, sessionOptions } | { kind: 'weblayout', payload }).
  • Implemented runFromAppDefAsync — inlines all initialization logic (locale, session management, runtime-map creation, toolbar parsing, selection recovery). sessionWasReused is inferred from options.sessionWasReused ?? !!options.session.
  • runAsync refactored to delegate through loadResourceAsync + runFromAppDefAsync for backward compatibility.

New standalone loadViewerResourceAsync function (src/actions/init-mapguide.ts)

  • Exported non-dispatchable async function loadViewerResourceAsync(cmd: DefaultViewerInitCommand, options: IInitAsyncOptions): Promise<LoadedResource>.
  • Resource loading is fully decoupled from any Redux thunk or dispatch; callers attach the client via cmd.attachClient(...) and then call this function directly.

New initAppFromAppDef thunked action (src/actions/init.ts)

  • New exported action with signature initAppFromAppDef(cmd: IViewerInitCommand, options: IInitAppLayout, appDef: ApplicationDefinition, viewer: IMapProviderContext): ReduxThunkedAction.
  • Calls cmd.runFromAppDefAsync, applies payload overrides (initialView, initialActiveMap, appSettings merge) via the extracted applyInitPayloadOverrides helper, dispatches INIT_APP, and calls onInit(viewer).
  • INIT_ERROR is dispatched if runFromAppDefAsync rejects.

initLayout removed (src/actions/init.ts)

  • initLayout is removed entirely. processAndDispatchInitError is now exported.

app.tsx (src/containers/app.tsx)

  • IAppProps.initCommand type narrowed from IViewerInitCommand to DefaultViewerInitCommand so that loadViewerResourceAsync can be called directly.
  • IAppDispatch.initLayout removed from the interface.
  • useConfiguredAgentUri() and useConfiguredAgentKind() hooks are called at the component body level to obtain agent config from Redux state.
  • The useEffect calls loadViewerResourceAsync(initCommand, opts) directly (no inline thunk), attaches the client beforehand if agentUri/agentKind are available, then dispatches initAppFromAppDef for AppDef resources or INIT_APP inline for WebLayout resources.

Public API (src/entries/library.tsx)

  • initAppFromAppDef added to library exports.
// Before: required a resource URL/ID routed through initLayout
dispatch(initLayout(initCommand, viewer, { resourceId: "Library://App.ApplicationDefinition", ... }));

// After: can drive init directly from a pre-loaded appdef
const appDef: ApplicationDefinition = await loadMyAppDef();
dispatch(initAppFromAppDef(cmd, options, appDef, viewer));

Tests

  • 7 new cases for initAppFromAppDef in test/actions/init.spec.ts: INIT_APP dispatch, INIT_ERROR on rejection, initialView/initialActiveMap overrides, onInit callback, appSettings merge, and client attachment behavior.
  • 4 new cases for DefaultViewerInitCommand.runFromAppDefAsync in test/actions/init-mapguide.spec.ts: payload structure from a real stateless AppDef with a subject-layer map, and sessionWasReused inference logic (no session, provided session without flag, provided session with explicit false).
  • Updated test/containers/app-component.spec.tsx: hooks mock includes useConfiguredAgentUri/useConfiguredAgentKind; verifies that loadResourceAsync is called with the correct init options built from URL state and props, and that initAppFromAppDef is subsequently dispatched with the loaded appDef.

Copilot AI and others added 2 commits May 5, 2026 16:18
…omAppDefAsync method

Agent-Logs-Url: https://github.com/jumpinjackie/mapguide-react-layout/sessions/89334cd8-f975-4147-bb71-9fd2c4256113

Co-authored-by: jumpinjackie <563860+jumpinjackie@users.noreply.github.com>
…promise from initAppFromAppDef thunk, update tests to use await

Agent-Logs-Url: https://github.com/jumpinjackie/mapguide-react-layout/sessions/89334cd8-f975-4147-bb71-9fd2c4256113

Co-authored-by: jumpinjackie <563860+jumpinjackie@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor init command to be a dispatchable init action Refactor init command to be a dispatchable init action May 5, 2026
Copilot AI requested a review from jumpinjackie May 5, 2026 16:24
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Bundle Size Report ✅ PASS

Metric Size Gzip Brotli
Total 10.21 MB 2.29 MB 1.63 MB
Diff +6.01 KB 🔴 ↑ +1.42 KB 🔴 ↑ +1.30 KB 🔴 ↑
Change +0.06% +0.06% +0.08%

Top Changes

File Before After Diff
~ viewer-debug.js 200.83 KB 201.79 KB +980.00 B 🔴 ↑
~ viewer.js 122.73 KB 123.08 KB +355.00 B 🔴 ↑

Generated by build-size-diff Commit: e794089

@coveralls
Copy link
Copy Markdown

coveralls commented May 6, 2026

Coverage Status

coverage: 59.808% (+0.1%) from 59.676% — copilot/refactor-init-command-to-dispatchable-action into master

@jumpinjackie
Copy link
Copy Markdown
Owner

@copilot The intent was correct (a new dispatchable action that takes an appdef payload), but the implementation was completely wrong.

For one, this new initAppFromAppDef action is only referenced by tests, but not in actual app code. I wanted the existing init infrastructure to fetch the appdef and then route through to this new action with the fetched appdef.

Secondly, I wanted this new action to not take an IViewerInitCommand argument. This new IViewerInitCommand.runFromAppDefAsync method you have delegated the run logic to from within this new initAppFromAppDef action, I was expecting you to inline all the implementation logic.

Copilot AI and others added 2 commits May 9, 2026 01:37
…tCommand internally, route via setViewer/app.tsx

Agent-Logs-Url: https://github.com/jumpinjackie/mapguide-react-layout/sessions/1b7c55cb-ad79-485f-a19a-acda593349d7

Co-authored-by: jumpinjackie <563860+jumpinjackie@users.noreply.github.com>
…ve comments, use afterEach for spy cleanup

Agent-Logs-Url: https://github.com/jumpinjackie/mapguide-react-layout/sessions/1b7c55cb-ad79-485f-a19a-acda593349d7

Co-authored-by: jumpinjackie <563860+jumpinjackie@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 9, 2026

@copilot The intent was correct (a new dispatchable action that takes an appdef payload), but the implementation was completely wrong.

For one, this new initAppFromAppDef action is only referenced ...

Addressed in commits 357006d and 0206d72.

The key changes:

  1. initAppFromAppDef no longer takes a cmd parameter — it creates a DefaultViewerInitCommand internally and calls cmd.runFromAppDefAsync(appDef, opts) directly. The full dispatch/override/onInit logic is inlined in the action body.

  2. Routing through the existing init infrastructure:

    • DefaultViewerInitCommand gains an optional setViewer method (on the IViewerInitCommand interface). When initLayout calls cmd.setViewer(viewer), the new dispatchOrProcess private helper routes fetched appdefs through initAppFromAppDef instead of processing them inline — this applies to the appdef.json, URL-to-appdef, and function-resourceId cases.
    • For the function-resourceId case specifically, app.tsx also fetches the appdef upfront and dispatches initAppFromAppDef directly, bypassing initLayout entirely.
  3. runFromAppDefAsync is removed from IViewerInitCommand (kept as a concrete method on DefaultViewerInitCommand, used internally by initAppFromAppDef).

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • storybook.js.org
    • Triggering command: /opt/hostedtoolcache/node/24.14.1/x64/bin/node /opt/hostedtoolcache/node/24.14.1/x64/bin/node /home/REDACTED/work/mapguide-react-layout/mapguide-react-layout/node_modules/storybook/bin/index.cjs build -c .storybook (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@jumpinjackie
Copy link
Copy Markdown
Owner

@copilot I am still not seeing the material gains with this refactoring you have done. Let me restate the high-level goals:

I want a new thunked action initAppFromAppDef with this signature:

export function initAppFromAppDef(cmd: IViewerInitCommand, options: IInitAppLayout, appDef: ApplicationDefinition, viewer: IMapProviderContext): ReduxThunkedAction {
    return (dispatch, getState) => {
        // Given the cmd, viewer, options and appDef, perform all the required initialization to assemble the final
        // payload for the INIT_APP action to be dispatched.
        const payload = ...;
        dispatch({
            type: ActionType.INIT_APP,
            payload
        });
    };
}

The existing initLayout thunked action should be replaced with initAppFromAppDef at all current call sites.

Our tests around this thunked action should verify the expected redux store state after a dispatched INIT_APP from a given source appDef + extra parameters.

The purpose of this refactoring is to pave way for #217 since our new thunked action takes the prepared appDef, whether this appDef went through deArrayification or not is irrelevant to this new thunked action. That way we can easily verify that a dispatched init of this new thunked action with an appDef that did not go through deArrayification produces the same redux store state as an appDef that did go through deArrayification. This way, we have greater confidence in ripping out (or code-split out) the hefty deArrayification code as part of #217 once we have confirmation that both appDefs yield the same init result through this new thunked action.

Pls start from the original branching point again and re-implement this change with this revised understanding in mind.

Copilot AI and others added 2 commits May 11, 2026 11:58
…/runFromAppDefAsync on IViewerInitCommand

Agent-Logs-Url: https://github.com/jumpinjackie/mapguide-react-layout/sessions/1e1794cd-9272-4502-9a5f-56de9ed7cf56

Co-authored-by: jumpinjackie <563860+jumpinjackie@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 11, 2026

@copilot I am still not seeing the material gains with this refactoring you have done. Let me restate the high-level goals:

I want a new thunked action initAppFromAppDef with this signature:
...

Implemented in commit eb694b6.

The new initAppFromAppDef action has the exact signature requested:

export function initAppFromAppDef(
    cmd: IViewerInitCommand,
    options: IInitAppLayout,
    appDef: ApplicationDefinition,
    viewer: IMapProviderContext
): ReduxThunkedAction

All initialization logic (locale init, session management, runtime-map creation, toolbar parsing, payload overrides, onInit callback) is inlined directly in runFromAppDefAsync on DefaultViewerInitCommand — no delegation to a separate method on the interface other than what the interface itself defines.

Routing through existing init infrastructure: IViewerInitCommand gains two new required methods:

  • loadResourceAsync(options) — fetches the resource (all cases: appdef.json, URL, MapGuide ApplicationDefinition, WebLayout, function), manages the session, and returns a LoadedResource discriminated union ({ kind: 'appdef', appDef, sessionOptions } | { kind: 'weblayout', payload })
  • runFromAppDefAsync(appDef, options) — builds the full IInitAppActionPayload from a pre-loaded ApplicationDefinition

initLayout now calls cmd.loadResourceAsync and, for the AppDef result, dispatches initAppFromAppDef directly. runAsync is kept for backward compat and simply delegates to loadResourceAsync + runFromAppDefAsync.

Tests verify the expected redux store state: 7 tests for initAppFromAppDef (INIT_APP dispatch, INIT_ERROR on rejection, initialView/initialActiveMap overrides, onInit callback, appSettings merge, client attachment) and 4 tests for DefaultViewerInitCommand.runFromAppDefAsync (payload structure from stateless AppDef, sessionWasReused inference logic).

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • storybook.js.org
    • Triggering command: /opt/hostedtoolcache/node/24.14.1/x64/bin/node /opt/hostedtoolcache/node/24.14.1/x64/bin/node /home/REDACTED/work/mapguide-react-layout/mapguide-react-layout/node_modules/storybook/bin/index.cjs build -c .storybook (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@jumpinjackie
Copy link
Copy Markdown
Owner

@copilot I wanted the initLayout thunked action gone and its call-site to be refactored to dispatch the replacement thunked action instead.

…after loadResourceAsync

Agent-Logs-Url: https://github.com/jumpinjackie/mapguide-react-layout/sessions/4d58274d-bf3a-4e6b-9d47-a106d30f3900

Co-authored-by: jumpinjackie <563860+jumpinjackie@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 11, 2026

@copilot I wanted the initLayout thunked action gone and its call-site to be refactored to dispatch the replacement thunked action instead.

Done in commit d061c7b.

initLayout is removed entirely. The useEffect in app.tsx now dispatches an inline thunk that calls initCommand.loadResourceAsync(opts) directly, then routes the result — dispatching initAppFromAppDef(initCommand, sessionOptions, appDef, viewer) for AppDef resources and INIT_APP inline for WebLayout resources.

IAppDispatch.initLayout is also removed, and processAndDispatchInitError is now exported from init.ts so app.tsx can use it for error handling.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • storybook.js.org
    • Triggering command: /opt/hostedtoolcache/node/24.14.1/x64/bin/node /opt/hostedtoolcache/node/24.14.1/x64/bin/node /home/REDACTED/work/mapguide-react-layout/mapguide-react-layout/node_modules/storybook/bin/index.cjs build -c .storybook (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@jumpinjackie
Copy link
Copy Markdown
Owner

@copilot I don't like this inline thunked action replacement. Is this solely because the loading of the appDef is tied to needing to call the added loadResourceAsync() on the IViewerInitCommand? If so, then rip that method out to its own non-dispatchable function. I want the resource/appdef loading part fully decoupled from any thunked action dispatch.

…adViewerResourceAsync

Agent-Logs-Url: https://github.com/jumpinjackie/mapguide-react-layout/sessions/a9a9b287-70dc-438c-93c6-a2f25fca405b

Co-authored-by: jumpinjackie <563860+jumpinjackie@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 11, 2026

@copilot I don't like this inline thunked action replacement. Is this solely because the loading of the appDef is tied to needing to call the added loadResourceAsync() on the IViewerInitCommand? If so...

Done in commit 59ff05b.

loadResourceAsync is removed from IViewerInitCommand and ViewerInitCommand. It's now a standalone exported function loadViewerResourceAsync(cmd: DefaultViewerInitCommand, options) in init-mapguide.ts, with no dispatch involvement.

app.tsx no longer uses an inline thunk — the useEffect reads agentUri/agentKind directly via the existing useConfiguredAgentUri()/useConfiguredAgentKind() hooks, attaches the client, then calls loadViewerResourceAsync(initCommand, opts) as a plain async call. For AppDef results it dispatches initAppFromAppDef(...); for WebLayout it dispatches INIT_APP inline.

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.

Refactor init command to be a dispatchable init action

3 participants