Initial page-load js (12.49 MB raw / 3,783 KB gzip) --> 2.62 MB raw (790 KB gzip)#4050
Open
jbecke wants to merge 1 commit into
Open
Initial page-load js (12.49 MB raw / 3,783 KB gzip) --> 2.62 MB raw (790 KB gzip)#4050jbecke wants to merge 1 commit into
jbecke wants to merge 1 commit into
Conversation
The entry chunk bundled nearly the whole app: every block UI (pdf, code, canvas, chat, ...), livekit-client + the Krisp noise filter, Datadog, PostHog, loro-crdt, the emoji datasets, and two ~700KB generated zod schema files all loaded before first paint. Initial JS drops from 12.49MB raw / 3,783KB gzip to 2.62MB raw / 790KB gzip. Block components - All block definitions now declare lazy() components (block-image and block-project already did); definitions stay eager for the registry metadata, the component trees split into route-style chunks. - block-md's definition loads @service-sync/source (loro) inside load(). Call stack (livekit-client + @livekit/krisp-noise-filter, the two largest dependencies) - New channel/Call/livekit-loader.ts: lazy loaders plus compile-time- checked mirrors of the livekit string/protobuf enums so call state can be tracked without the SDK. - CallContext creates the LivekitJsCallController (and loads livekit + Krisp) on first connect; device enumeration and noise suppression resolve the SDKs lazily; CallAudioSink/use-call use the enum mirrors. - Root/TauriProvider/sidebar import call modules directly instead of the @channel/Call barrel, which kept CallOverlay/InCallPanel eager. - callkit-drawer-theme: replaced colorjs.io with the closed-form OKLCH -> sRGB conversion (same math, zero dependency). Observability / analytics - observability package now loads the Datadog SDKs via dynamic import in init(); pre-init calls fall back to the console exactly as before. - posthog-js loads lazily behind a queueing facade; capture/identify/ reset/onFeatureFlags replay once attached, flag reads report undefined until loaded (same answers PostHog gives before its flag fetch). Lexical / markdown stack - initializeLexical() imports node classes from individual modules, not the @lexical-core barrel (which dragged node-list/@lexical/table and CustomCodeNode/prismjs). - Decorator-shared commands moved to plugins/commands.ts; decorators no longer import the plugins barrel (which pulled pluginManager -> loro). - LexicalWrapperContext split: light context in wrapperContext.ts. - pluginManager loads the loro undo binding lazily (collab callers have loro loaded already, so this resolves in a microtask). - DiffNode.handleAccept loads @lexical/markdown + transformers lazily. - StaticMarkdown consumers in the boot path (DiffInsert, DocumentPreview, entity extractors, search content) lazy-load it; EmojiMenu (fuse.js + emoji datasets) is lazy inside MarkdownShell; DocumentCard lazy-loads ChannelMessageThreadCard (the whole channel Message tree). Boot-path import graph fixes - block-pdf URL_PARAMS moved to @block-pdf/constants; isThreadPlaceable moved to type/placeables — light consumers (DocumentPreview, citations, notification navigation, the storage client) no longer pull the entire PDF viewer. - Removed the katex/pdfjs manualChunks: both are dynamic-only now, and the manual chunk made rollup hoist shared commonjs helpers into the pdfjs chunk, statically dragging it into the entry. - componentRegistry lazy-loads Settings, ChannelCompose, EmailCompose, ComposeTask, NotificationsPlayground (removed from the notifications barrel); Layout lazy-loads IosShareSheet (iOS-gated) and AutomationComposer (open-gated, signal split into its own module); GlobalShareModal lazy-loads ShareModal; share-button refetch registry and drawer DOM helper split out of ShareButton.tsx. - instructions-md query loads the editor machinery inside the query fn. Generated zod schemas (~900KB source, not tree-shakeable) - storage client loads saveDocumentHandlerResponse lazily (all call sites are post-await). - notifications build their schema lazily with an unvalidated fallback until the chunk loads — the same fallback the parse-failure path already took. Tests: 952 passed; the 2 websocket reconnect failures also fail on the base commit (pre-existing flakes). https://claude.ai/code/session_01B2vhy7mk22vaiTvzEKPVhw
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR refactors the codebase to improve code-splitting and reduce initial bundle size through systematic lazy-loading of components and SDKs. It converts 14 block definitions, major UI components (AutomationComposer, ShareModal, InteractiveOnboarding), and Lexical markdown editors from static imports to lazy-loaded variants wrapped in Suspense boundaries. The changes reorganize Lexical module imports to use specific entry points instead of barrel exports, centralize command definitions into a dedicated commands module, and extract context types into a dedicated wrapper context module. A new livekit-loader module provides lazy access to LiveKit and Krisp SDKs with compile-time-safe enum mirrors. External SDKs including PostHog and Datadog are converted to lazy-loaded facades that queue operations until the real implementations are available. Utility logic including PDF URL parameters, placeable types, share button refetch callbacks, and notification schema validation is extracted into dedicated modules. Additional changes remove the colorjs.io dependency by inlining color conversion math, optimize Vite chunk configuration, defer sync service imports, and update import paths throughout to reference specific modules instead of barrel exports. |
jbecke
changed the title
jbecke
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
js/app/packages/queries/storage/instructions-md.ts (1)
55-67: ⚡ Quick winParallelize the raw-state fetch and lazy imports to avoid a request→chunk waterfall.
At Line 55 and Line 64, these independent awaits are sequential right now. Starting both at once trims first-load latency for this query path.
♻️ Proposed refactor
- const rawState = await syncServiceClient.getRaw({ - documentId: id, - }); - - // Loaded lazily: this is the only boot-path consumer of the editor - // machinery, which would otherwise land in the initial bundle. - const [ - { createLexicalWrapper }, - { getTextContent, initializeEditorWithState }, - ] = await Promise.all([ - import('`@core/component/LexicalMarkdown/context/LexicalWrapperContext`'), - import('`@core/component/LexicalMarkdown/utils`'), - ]); + const rawStatePromise = syncServiceClient.getRaw({ documentId: id }); + const lexicalDepsPromise = Promise.all([ + import('`@core/component/LexicalMarkdown/context/LexicalWrapperContext`'), + import('`@core/component/LexicalMarkdown/utils`'), + ]); + + const [ + rawState, + [{ createLexicalWrapper }, { getTextContent, initializeEditorWithState }], + ] = await Promise.all([rawStatePromise, lexicalDepsPromise]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@js/app/packages/queries/storage/instructions-md.ts` around lines 55 - 67, The current code awaits syncServiceClient.getRaw (rawState) before starting the lazy imports, creating a request→chunk waterfall; instead start both operations concurrently by kicking off the raw fetch promise and the import Promise.all at the same time, then await both promises (referencing rawState from the resolved syncServiceClient.getRaw promise and the imported symbols createLexicalWrapper, getTextContent, initializeEditorWithState from the Promise.all import). Ensure you still use the same identifiers (rawState, syncServiceClient.getRaw, createLexicalWrapper, getTextContent, initializeEditorWithState, initializeEditorWithState) so downstream code is unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@js/app/packages/app/component/Layout.tsx`:
- Around line 174-176: The IosShareSheet lazy component is rendered directly
under the outer Suspense and can suspend the whole authenticated subtree; wrap
the IosShareSheet component in a local Suspense boundary (similar to the
AutomationComposer pattern) so its loading only affects itself. Locate the Show
block using getPlatform() === 'ios' and replace the direct <IosShareSheet />
with a local <Suspense fallback={null}> (or appropriate minimal fallback) that
contains <IosShareSheet /> to isolate its suspension.
In `@js/app/packages/app/component/next-soup/utils.ts`:
- Line 12: openEntityInNewTab currently writes legacy PDF query keys
(search_page, search_raw_query, search_highlight_terms, search_snippet) when
building PDF new-tab URLs, but navigateToLocation and block-pdf expect canonical
PDF_PARAMS keys (pdf_search_page, pdf_search_raw_query,
pdf_search_highlight_terms, pdf_search_snippet). Fix openEntityInNewTab (pdf
case) to set entityUrl.searchParams using PDF_PARAMS.searchPage,
PDF_PARAMS.searchRawQuery, PDF_PARAMS.searchHighlightTerms and
PDF_PARAMS.searchSnippet instead of the hardcoded legacy strings so the created
URL matches parseLocationFromUrl and navigateToLocation.
In `@js/app/packages/channel/Call/callkit-drawer-theme.ts`:
- Around line 182-186: The green-channel coefficient in oklchToLinearSrgb is
wrong: in the returned array (the second element mapping LMS to linear sRGB)
replace the term using -0.7034186147 * s with Ottosson’s correct coefficient
-0.3413193965 * s so the matrix row becomes -1.2684380046 * l + 2.6097574011 * m
- 0.3413193965 * s; update that coefficient in the function oklchToLinearSrgb to
correct the LMS→linear-sRGB conversion.
In `@js/app/packages/core/component/LexicalMarkdown/plugins/pluginManager.ts`:
- Around line 41-44: The dynamic import in the history() block may reject and
cause an unhandled promise; wrap the import promise with error handling so
failures are caught and handled: add a .catch(...) (or try/catch around an async
import) to the import('../collaboration/undo') call and in the catcher log or
no-op (using existing logging mechanism) and ensure you do not change cleanup
when the import fails; keep references to registerLoroHistory,
loroManager.getDoc(), timeGap, cleanup and cancelled and ensure that if the
import fails you return/leave cleanup undefined and avoid calling
registerLoroHistory for editor.
In `@js/app/packages/entity/src/extractors-search/search-content.tsx`:
- Around line 46-48: The Suspense wrapper around StaticMarkdown currently has no
fallback which can cause a blank snippet during loading; update the Suspense
(wrapping StaticMarkdown in search-content.tsx) to include an appropriate
fallback prop (e.g., a lightweight skeleton/spinner or a condensed text
placeholder) so users see a stable UI while trimmedContent() and theme()
resolve; ensure the fallback visually matches the snippet size and replace the
current empty Suspense usage with Suspense fallback={...} referencing the same
StaticMarkdown component context.
In `@js/app/packages/notifications/notification-source.ts`:
- Around line 215-217: The fire-and-forget call to
loadUnifiedNotificationSchema() can produce an unhandled rejection if the
dynamic import fails; change the invocation at the top-level (the current "void
loadUnifiedNotificationSchema();") to handle errors explicitly by attaching a
.catch(...) (or awaiting inside an async init) and log or safely swallow the
error. Specifically, replace the bare call with something like: void
loadUnifiedNotificationSchema().catch(err => /* use existing logger or
console.error with a clear message referencing loadUnifiedNotificationSchema */)
so any chunk-load failures are caught and reported.
In `@js/app/packages/notifications/types.ts`:
- Around line 27-33: The dynamic-import memoization in
loadUnifiedNotificationSchema() keeps schemaPromise set on rejection and drops
the error, so reset schemaPromise and clear loadedSchema when the import/build
fails and re-throw the error to avoid a permanently failed cache; specifically,
change the Promise chain for schemaPromise (and
buildUnifiedNotificationSchema(m) usage) to include a .catch(err => {
schemaPromise = undefined; loadedSchema = null; throw err; }) so subsequent
calls can retry, and update the caller that uses void
loadUnifiedNotificationSchema() to attach a .catch(...) to avoid unhandled
rejections.
In `@js/app/packages/observability/src/index.ts`:
- Around line 5-9: Replace the loose any index signature on the User interface
and other observability payload types with unknown and narrow at consumption
sites: change "interface User { id: string; email: string; [key: string]: any }"
to use "[key: string]: unknown", update analogous signatures in logger.ts and
impl.ts to unknown, then update code that reads these dynamic properties (e.g.,
inside the logger creation functions and the implementation code around the
consumer at impl.ts line ~96) to type-check/cast or validate the expected shape
before accessing fields (use type predicates, safe property checks, or explicit
casts) so payload handling remains type-safe.
In `@js/app/packages/service-clients/service-storage/client.ts`:
- Around line 157-162: getSavedDocumentMetadataSchema currently memoizes a
failing dynamic import forever; change it to catch errors from
import('./generated/zod'), reset savedDocumentMetadataSchemaPromise to undefined
(so a transient failure doesn't get cached) and rethrow the error so callers can
handle it; then update pdfSave, simpleSave, and simpleSaveText to wrap the await
getSavedDocumentMetadataSchema() in a try/catch and convert import/schema-load
errors into the recoverable Result failure path before calling .safeParse (or
bail early), ensuring you reference savedDocumentMetadataSchemaPromise,
getSavedDocumentMetadataSchema, and the three save functions when making the
changes.
In `@js/lexical-core/nodes/DiffNode.ts`:
- Around line 113-142: Capture the node key before the lazy imports and
re-resolve the node inside editor.update to avoid operating on a
possibly-stale/captured `this` (e.g., in DiffNode.handleAccept do const nodeKey
= this.getKey(); then inside editor.update call const node =
$getNodeByKey(nodeKey) and bail if null before calling node.getChildren(),
node.clear(), $convertFromMarkdownString(..., node, ...), and
$unwrapNode(node)); also append a .catch(...) to the Promise.all(...) import
chain to handle and log import errors so you don't leave unhandled promise
rejections.
---
Nitpick comments:
In `@js/app/packages/queries/storage/instructions-md.ts`:
- Around line 55-67: The current code awaits syncServiceClient.getRaw (rawState)
before starting the lazy imports, creating a request→chunk waterfall; instead
start both operations concurrently by kicking off the raw fetch promise and the
import Promise.all at the same time, then await both promises (referencing
rawState from the resolved syncServiceClient.getRaw promise and the imported
symbols createLexicalWrapper, getTextContent, initializeEditorWithState from the
Promise.all import). Ensure you still use the same identifiers (rawState,
syncServiceClient.getRaw, createLexicalWrapper, getTextContent,
initializeEditorWithState, initializeEditorWithState) so downstream code is
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c3bc22db-706c-4478-bdba-5528d964a5ff
📒 Files selected for processing (87)
js/app/packages/app/component/Launcher.tsxjs/app/packages/app/component/Layout.tsxjs/app/packages/app/component/Root.tsxjs/app/packages/app/component/app-sidebar/active-call-widget.tsxjs/app/packages/app/component/app-sidebar/sidebar.tsxjs/app/packages/app/component/global-share-modal/GlobalShareModal.tsxjs/app/packages/app/component/interactive-onboarding/InteractiveOnboardingModal.tsxjs/app/packages/app/component/next-soup/soup-view/SoupEntityActionDrawer.tsxjs/app/packages/app/component/next-soup/utils.tsjs/app/packages/app/component/split-layout/componentRegistry.tsxjs/app/packages/app/lib/analytics/analytics.tsjs/app/packages/app/vite.base.tsjs/app/packages/block-automation/component/AutomationComposer.tsxjs/app/packages/block-automation/component/automationComposerState.tsjs/app/packages/block-automation/component/index.tsjs/app/packages/block-automation/definition.tsjs/app/packages/block-call/definition.tsjs/app/packages/block-canvas/definition.tsjs/app/packages/block-channel/definition.tsjs/app/packages/block-chat/definition.tsjs/app/packages/block-code/definition.tsjs/app/packages/block-company/definition.tsjs/app/packages/block-contact/definition.tsjs/app/packages/block-email/definition.tsjs/app/packages/block-md/definition.tsjs/app/packages/block-pdf/constants.tsjs/app/packages/block-pdf/definition.tsjs/app/packages/block-pdf/signal/location.tsjs/app/packages/block-pdf/store/comments/freeComments.tsjs/app/packages/block-pdf/type/coParse.tsjs/app/packages/block-pdf/type/placeables.tsjs/app/packages/block-unknown/definition.tsjs/app/packages/block-video/definition.tsjs/app/packages/channel/Call/CallAudioSink.tsxjs/app/packages/channel/Call/CallContext.tsxjs/app/packages/channel/Call/callkit-drawer-theme.tsjs/app/packages/channel/Call/livekit-loader.tsjs/app/packages/channel/Call/use-call.tsjs/app/packages/core/component/DocumentPreview.tsxjs/app/packages/core/component/LexicalMarkdown/builder/MarkdownShell.tsxjs/app/packages/core/component/LexicalMarkdown/citationsUtils.tsjs/app/packages/core/component/LexicalMarkdown/component/decorator/ContactMention.tsxjs/app/packages/core/component/LexicalMarkdown/component/decorator/DateMention.tsxjs/app/packages/core/component/LexicalMarkdown/component/decorator/DiffInsert.tsxjs/app/packages/core/component/LexicalMarkdown/component/decorator/DocumentCard.tsxjs/app/packages/core/component/LexicalMarkdown/component/decorator/DocumentMention.tsxjs/app/packages/core/component/LexicalMarkdown/component/decorator/Equation.tsxjs/app/packages/core/component/LexicalMarkdown/component/decorator/GroupMention.tsxjs/app/packages/core/component/LexicalMarkdown/component/decorator/HorizontalRule.tsxjs/app/packages/core/component/LexicalMarkdown/component/decorator/MarkdownImage.tsxjs/app/packages/core/component/LexicalMarkdown/component/decorator/MarkdownVideo.tsxjs/app/packages/core/component/LexicalMarkdown/component/decorator/Snapshot.tsxjs/app/packages/core/component/LexicalMarkdown/component/decorator/ThemeMention.tsxjs/app/packages/core/component/LexicalMarkdown/component/decorator/UnknownMention.tsxjs/app/packages/core/component/LexicalMarkdown/component/decorator/UserMention.tsxjs/app/packages/core/component/LexicalMarkdown/component/dom-factory/diff-factory.tsxjs/app/packages/core/component/LexicalMarkdown/context/LexicalWrapperContext.tsxjs/app/packages/core/component/LexicalMarkdown/context/wrapperContext.tsjs/app/packages/core/component/LexicalMarkdown/init.tsjs/app/packages/core/component/LexicalMarkdown/plugins/commands.tsjs/app/packages/core/component/LexicalMarkdown/plugins/document-metadata/documentMetadataPlugin.tsjs/app/packages/core/component/LexicalMarkdown/plugins/katex/katexPlugin.tsjs/app/packages/core/component/LexicalMarkdown/plugins/media/mediaPlugin.tsjs/app/packages/core/component/LexicalMarkdown/plugins/mentions/mentionsPlugin.tsjs/app/packages/core/component/LexicalMarkdown/plugins/pluginManager.tsjs/app/packages/core/component/LexicalMarkdown/utils.tsjs/app/packages/core/component/TopBar/ShareButton.tsxjs/app/packages/core/component/TopBar/shareButtonRefetch.tsjs/app/packages/core/component/TopBar/shareDrawer.tsjs/app/packages/entity/src/extractors-search/search-content.tsxjs/app/packages/entity/src/extractors/entity-title.tsxjs/app/packages/notifications/index.tsjs/app/packages/notifications/notification-navigation.tsjs/app/packages/notifications/notification-platform.tsjs/app/packages/notifications/notification-source.tsjs/app/packages/notifications/types.tsjs/app/packages/observability/src/actionTracker.tsjs/app/packages/observability/src/impl.tsjs/app/packages/observability/src/index.tsjs/app/packages/observability/src/logger.tsjs/app/packages/observability/src/routingTracker.tsjs/app/packages/observability/src/shared.tsjs/app/packages/queries/storage/instructions-md.tsjs/app/packages/service-clients/service-storage/client.tsjs/app/packages/service-clients/service-storage/util/refetchResources.tsjs/app/packages/tauri/src/TauriProvider.tsxjs/lexical-core/nodes/DiffNode.ts
💤 Files with no reviewable changes (1)
- js/app/packages/notifications/index.ts
Comment on lines
+174
to
+176
| <Show when={getPlatform() === 'ios'}> | ||
| <IosShareSheet /> | ||
| </Show> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify where IosShareSheet is mounted relative to Suspense boundaries.
rg -n -C3 "IosShareSheet|<Suspense>|</Suspense>" js/app/packages/app/component/Layout.tsxRepository: macro-inc/macro
Length of output: 1847
🏁 Script executed:
#!/bin/bash
# Inspect surrounding Suspense boundaries and the IosShareSheet render location
nl -ba js/app/packages/app/component/Layout.tsx | sed -n '140,200p'Repository: macro-inc/macro
Length of output: 100
🏁 Script executed:
#!/bin/bash
# Show Layout.tsx section around the Suspense boundaries and IosShareSheet
sed -n '130,200p' js/app/packages/app/component/Layout.tsxRepository: macro-inc/macro
Length of output: 2095
🏁 Script executed:
#!/bin/bash
# Find all Suspense usages (and whether they specify a fallback) in this file
rg -n "<Suspense" js/app/packages/app/component/Layout.tsxRepository: macro-inc/macro
Length of output: 180
Wrap IosShareSheet in a local <Suspense> boundary.
IosShareSheet is a lazy component rendered directly under the outer <Suspense> (no local boundary), so its first unresolved load can suspend the rest of the authenticated subtree. Isolate it like the existing AutomationComposer pattern.
Proposed fix
- <Show when={getPlatform() === 'ios'}>
- <IosShareSheet />
- </Show>
+ <Show when={getPlatform() === 'ios'}>
+ <Suspense>
+ <IosShareSheet />
+ </Suspense>
+ </Show>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Show when={getPlatform() === 'ios'}> | |
| <IosShareSheet /> | |
| </Show> | |
| <Show when={getPlatform() === 'ios'}> | |
| <Suspense> | |
| <IosShareSheet /> | |
| </Suspense> | |
| </Show> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@js/app/packages/app/component/Layout.tsx` around lines 174 - 176, The
IosShareSheet lazy component is rendered directly under the outer Suspense and
can suspend the whole authenticated subtree; wrap the IosShareSheet component in
a local Suspense boundary (similar to the AutomationComposer pattern) so its
loading only affects itself. Locate the Show block using getPlatform() === 'ios'
and replace the direct <IosShareSheet /> with a local <Suspense fallback={null}>
(or appropriate minimal fallback) that contains <IosShareSheet /> to isolate its
suspension.
| import { URL_PARAMS as EMAIL_PARAMS } from '@block-email/constants'; | ||
| import { URL_PARAMS as MD_PARAMS } from '@block-md/constants'; | ||
| import { URL_PARAMS as PDF_PARAMS } from '@block-pdf/signal/location'; | ||
| import { URL_PARAMS as PDF_PARAMS } from '@block-pdf/constants'; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Canonical PDF URL param definitions =="
rg -n --type=ts --type=tsx -C2 "pdf_search_page|pdf_search_raw_query|pdf_search_highlight_terms|pdf_search_snippet|URL_PARAMS\\.searchPage|URL_PARAMS\\.searchRawQuery|URL_PARAMS\\.searchHighlightTerms|URL_PARAMS\\.searchSnippet"
echo
echo "== Legacy literal key usage =="
rg -n --type=ts --type=tsx -C2 "'search_page'|\"search_page\"|'search_raw_query'|\"search_raw_query\"|'search_highlight_terms'|\"search_highlight_terms\"|'search_snippet'|\"search_snippet\""
echo
echo "== URL parsing/normalization sites for PDF params =="
rg -n --type=ts --type=tsx -C3 "parseLocationFromUrl|parseLocationFromBlockParams|goToLocationFromParams|URLSearchParams|get\\(.*search_page|get\\(.*pdf_search_page"Repository: macro-inc/macro
Length of output: 147
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Canonical PDF URL param definitions (PDF_PARAMS / URL_PARAMS members) =="
rg -n -g'*.ts' -g'*.tsx' -C2 \
"PDF_PARAMS|URL_PARAMS\\.(searchPage|searchRawQuery|searchHighlightTerms|searchSnippet)" \
js/app || true
echo
echo "== Legacy literal key usage (search_page / raw_query / highlight_terms / snippet) =="
rg -n -g'*.ts' -g'*.tsx' -C3 \
"['\"]search_page['\"]|['\"]search_raw_query['\"]|['\"]search_highlight_terms['\"]|['\"]search_snippet['\"]" \
js/app || true
echo
echo "== Usage of pdf_search_* keys =="
rg -n -g'*.ts' -g'*.tsx' -C2 \
"pdf_search_page|pdf_search_raw_query|pdf_search_highlight_terms|pdf_search_snippet" \
js/app || true
echo
echo "== URL parsing/normalization sites for PDF params =="
rg -n -g'*.ts' -g'*.tsx' -C3 \
"(parseLocationFromUrl|parseLocationFromBlockParams|goToLocationFromParams|URLSearchParams|get\\(.*search_page|get\\(.*pdf_search_page))" \
js/app || true
echo
echo "== Focus: utils.ts functions around openEntityInNewTab / navigateToLocation =="
FILE="js/app/packages/app/component/next-soup/utils.ts"
if [ -f "$FILE" ]; then
rg -n -g'*.ts' -g'*.tsx' "openEntityInNewTab|navigateToLocation" "$FILE" || true
echo "--- openEntityInNewTab context ---"
rg -n -g'*.ts' -g'*.tsx' "openEntityInNewTab" "$FILE" -C12 || true
echo "--- navigateToLocation context ---"
rg -n -g'*.ts' -g'*.tsx' "navigateToLocation" "$FILE" -C12 || true
else
echo "Missing file: $FILE" >&2
exit 1
fiRepository: macro-inc/macro
Length of output: 9124
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Where legacy keys are used across the repo =="
rg -n -g'*.ts' -g'*.tsx' \
"['\"]search_page['\"]|['\"]search_raw_query['\"]|['\"]search_highlight_terms['\"]|['\"]search_snippet['\"]" \
js/app | head -n 200
echo
echo "== Where canonical pdf_search_* keys are referenced =="
rg -n -g'*.ts' -g'*.tsx' \
"pdf_search_page|pdf_search_raw_query|pdf_search_highlight_terms|pdf_search_snippet" \
js/app | head -n 200
echo
echo "== Any normalization from legacy -> canonical in block-pdf package? =="
rg -n -g'*.ts' -g'*.tsx' \
"search_page|search_raw_query|search_highlight_terms|search_snippet|pdf_search_page|pdf_search_raw_query|pdf_search_highlight_terms|pdf_search_snippet" \
js/app/packages/block-pdf | head -n 400Repository: macro-inc/macro
Length of output: 1297
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) next-soup/utils.ts around openEntityInNewTab pdf block
FILE1="js/app/packages/app/component/next-soup/utils.ts"
echo "=== openEntityInNewTab PDF block (utils.ts) ==="
sed -n '190,270p' "$FILE1" | cat -n
echo
echo "=== navigateToLocation PDF block (utils.ts) ==="
sed -n '430,490p' "$FILE1" | cat -n
# 2) block-pdf constants and parsing
FILE2="js/app/packages/block-pdf/constants.ts"
echo
echo "=== block-pdf/constants.ts (URL_PARAMS) ==="
sed -n '1,120p' "$FILE2" | cat -n
FILE3="js/app/packages/block-pdf/component/Document.tsx"
echo
echo "=== block-pdf/Document.tsx query param reads ==="
# Find the exact searchParams usage region
rg -n "searchParams\\[URL_PARAMS" "$FILE3" | head -n 20
sed -n '500,580p' "$FILE3" | cat -n
FILE4="js/app/packages/block-pdf/signal/location.ts"
echo
echo "=== block-pdf/signal/location.ts parseLocationParams mapping ==="
rg -n "parseLocationParams\\(|URL_PARAMS\\." "$FILE4" | head -n 50
sed -n '360,460p' "$FILE4" | cat -n
# 3) Search for any legacy -> canonical fallback in block-pdf
echo
echo "=== block-pdf legacy key fallback checks ==="
rg -n -g'*.ts' -g'*.tsx' "search_page|search_raw_query|search_highlight_terms|search_snippet" js/app/packages/block-pdf || true
# 4) Rerun URL parsing helper location search (use simpler regex, avoid capturing group mistakes)
echo
echo "=== find URL parsing helpers mentioning search_page/pdf_search_page ==="
rg -n -g'*.ts' -g'*.tsx' "pdf_search_page|search_page" js/app/packages/block-pdf | head -n 100Repository: macro-inc/macro
Length of output: 15019
Fix PDF new-tab deep links to use canonical pdf_search_* query param keys.
openEntityInNewTab (pdf case) sets legacy keys (search_page, search_raw_query, search_highlight_terms, search_snippet), but navigateToLocation uses PDF_PARAMS.* (pdf_search_*), and block-pdf’s URL parsing (Document.tsx/parseLocationFromUrl) reads only URL_PARAMS.*. This mismatch will silently drop the target location in new-tab PDF links (no legacy fallback in block-pdf).
Update openEntityInNewTab to set entityUrl.searchParams using PDF_PARAMS.searchPage/searchRawQuery/searchHighlightTerms/searchSnippet instead of the hardcoded legacy strings. (js/app/packages/app/component/next-soup/utils.ts, openEntityInNewTab pdf case ~222-243)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@js/app/packages/app/component/next-soup/utils.ts` at line 12,
openEntityInNewTab currently writes legacy PDF query keys (search_page,
search_raw_query, search_highlight_terms, search_snippet) when building PDF
new-tab URLs, but navigateToLocation and block-pdf expect canonical PDF_PARAMS
keys (pdf_search_page, pdf_search_raw_query, pdf_search_highlight_terms,
pdf_search_snippet). Fix openEntityInNewTab (pdf case) to set
entityUrl.searchParams using PDF_PARAMS.searchPage, PDF_PARAMS.searchRawQuery,
PDF_PARAMS.searchHighlightTerms and PDF_PARAMS.searchSnippet instead of the
hardcoded legacy strings so the created URL matches parseLocationFromUrl and
navigateToLocation.
Comment on lines
+182
to
+186
| return [ | ||
| 4.0767416621 * l - 3.3077115913 * m + 0.2309699292 * s, | ||
| -1.2684380046 * l + 2.6097574011 * m - 0.7034186147 * s, | ||
| -0.0041960863 * l - 0.7034186147 * m + 1.707614701 * s, | ||
| ]; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
OKLab to linear sRGB matrix coefficients Ottosson
💡 Result:
OKLab to linear sRGB matrix coefficients (Ottosson / Oklab reference implementation), for linear-light sRGB (no gamma): linear_srgb = [r] [ 4.0767416621 -3.3077115913 0.2309699292] [l] [g] = [-1.2684380046 2.6097574011 -0.3413193965] [m] [b] [-0.0041960863 -0.7034186147 1.7076147010] [s] Where l,m,s are the LMS cube-root domain computed from Oklab as: l_ = L + 0.3963377774a + 0.2158037573b m_ = L - 0.1055613458a - 0.0638541728b s_ = L - 0.0894841775a - 1.2914855480b l = l_^3, m = m_^3, s = s_^3. These coefficients are explicitly given in Ottosson’s published reference code (and mirrored in implementations/libraries). [1][2][3]
Citations:
- 1: https://bottosson.github.io/posts/oklab/
- 2: https://github.com/bottosson/bottosson.github.io/blob/master/misc/ok_color.h
- 3: https://docs.rs/oklab/latest/src/oklab/lib.rs.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/channel/Call/callkit-drawer-theme.ts"
if [ ! -f "$FILE" ]; then echo "Missing $FILE"; exit 1; fi
nl -ba "$FILE" | sed -n '160,210p'Repository: macro-inc/macro
Length of output: 102
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/channel/Call/callkit-drawer-theme.ts"
nl -ba "$FILE" | sed -n '160,210p'Repository: macro-inc/macro
Length of output: 102
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/channel/Call/callkit-drawer-theme.ts"
nl -ba "$FILE" | sed -n '160,210p'Repository: macro-inc/macro
Length of output: 102
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/channel/Call/callkit-drawer-theme.ts"
nl -ba "$FILE" | sed -n '160,210p'Repository: macro-inc/macro
Length of output: 102
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/channel/Call/callkit-drawer-theme.ts"
nl -ba "$FILE" | sed -n '160,210p'Repository: macro-inc/macro
Length of output: 102
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/channel/Call/callkit-drawer-theme.ts"
test -f "$FILE" && echo "Found $FILE" || { echo "Missing $FILE"; exit 1; }
echo "---- lines 160-205 ----"
sed -n '160,205p' "$FILE" | awk '{print NR+159 ":" $0}'
echo "---- occurrences of -0.7034186147 ----"
rg -n --fixed-string "-0.7034186147" "$FILE" || true
echo "---- occurrences of -0.3413193965 ----"
rg -n --fixed-string "-0.3413193965" "$FILE" || trueRepository: macro-inc/macro
Length of output: 2820
Fix OKLCH→linear sRGB matrix coefficient for green channel
oklchToLinearSrgb() uses the wrong LMS→linear-sRGB coefficient for the green channel: line 184 has -0.7034186147 * s, but Ottosson’s reference uses -0.3413193965 for that term. This will skew all green values produced by this conversion.
🐛 Proposed fix
return [
4.0767416621 * l - 3.3077115913 * m + 0.2309699292 * s,
- -1.2684380046 * l + 2.6097574011 * m - 0.7034186147 * s,
+ -1.2684380046 * l + 2.6097574011 * m - 0.3413193965 * s,
-0.0041960863 * l - 0.7034186147 * m + 1.707614701 * s,
];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return [ | |
| 4.0767416621 * l - 3.3077115913 * m + 0.2309699292 * s, | |
| -1.2684380046 * l + 2.6097574011 * m - 0.7034186147 * s, | |
| -0.0041960863 * l - 0.7034186147 * m + 1.707614701 * s, | |
| ]; | |
| return [ | |
| 4.0767416621 * l - 3.3077115913 * m + 0.2309699292 * s, | |
| -1.2684380046 * l + 2.6097574011 * m - 0.3413193965 * s, | |
| -0.0041960863 * l - 0.7034186147 * m + 1.707614701 * s, | |
| ]; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@js/app/packages/channel/Call/callkit-drawer-theme.ts` around lines 182 - 186,
The green-channel coefficient in oklchToLinearSrgb is wrong: in the returned
array (the second element mapping LMS to linear sRGB) replace the term using
-0.7034186147 * s with Ottosson’s correct coefficient -0.3413193965 * s so the
matrix row becomes -1.2684380046 * l + 2.6097574011 * m - 0.3413193965 * s;
update that coefficient in the function oklchToLinearSrgb to correct the
LMS→linear-sRGB conversion.
Comment on lines
+41
to
+44
| void import('../collaboration/undo').then(({ registerLoroHistory }) => { | ||
| if (cancelled) return; | ||
| cleanup = registerLoroHistory(editor, loroManager.getDoc(), timeGap); | ||
| }); |
Contributor
There was a problem hiding this comment.
Handle dynamic import rejection in history() to avoid unhandled promise failures.
Line 41 starts a lazy import with only .then(...); if the chunk fails to load, this becomes an unhandled rejection and leaves history unregistered for that editor instance.
Suggested patch
- void import('../collaboration/undo').then(({ registerLoroHistory }) => {
- if (cancelled) return;
- cleanup = registerLoroHistory(editor, loroManager.getDoc(), timeGap);
- });
+ void import('../collaboration/undo')
+ .then(({ registerLoroHistory }) => {
+ if (cancelled) return;
+ cleanup = registerLoroHistory(editor, loroManager.getDoc(), timeGap);
+ })
+ .catch((error) => {
+ if (cancelled) return;
+ console.error('Failed to load collaboration undo module', error);
+ cleanup = registerHistory(editor, createEmptyHistoryState(), timeGap);
+ });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@js/app/packages/core/component/LexicalMarkdown/plugins/pluginManager.ts`
around lines 41 - 44, The dynamic import in the history() block may reject and
cause an unhandled promise; wrap the import promise with error handling so
failures are caught and handled: add a .catch(...) (or try/catch around an async
import) to the import('../collaboration/undo') call and in the catcher log or
no-op (using existing logging mechanism) and ensure you do not change cleanup
when the import fails; keep references to registerLoroHistory,
loroManager.getDoc(), timeGap, cleanup and cancelled and ensure that if the
import fails you return/leave cleanup undefined and avoid calling
registerLoroHistory for editor.
Comment on lines
+46
to
+48
| <Suspense> | ||
| <StaticMarkdown markdown={trimmedContent()} theme={theme()} /> | ||
| </Suspense> |
Contributor
There was a problem hiding this comment.
Add a Suspense fallback to prevent blank snippet flashes.
At Line 46, Suspense has no fallback, so the snippet can render empty while StaticMarkdown is still loading.
💡 Suggested patch
- <Suspense>
+ <Suspense fallback={<span class="italic text-ink-disabled">Loading…</span>}>
<StaticMarkdown markdown={trimmedContent()} theme={theme()} />
</Suspense>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Suspense> | |
| <StaticMarkdown markdown={trimmedContent()} theme={theme()} /> | |
| </Suspense> | |
| <Suspense fallback={<span class="italic text-ink-disabled">Loading…</span>}> | |
| <StaticMarkdown markdown={trimmedContent()} theme={theme()} /> | |
| </Suspense> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@js/app/packages/entity/src/extractors-search/search-content.tsx` around lines
46 - 48, The Suspense wrapper around StaticMarkdown currently has no fallback
which can cause a blank snippet during loading; update the Suspense (wrapping
StaticMarkdown in search-content.tsx) to include an appropriate fallback prop
(e.g., a lightweight skeleton/spinner or a condensed text placeholder) so users
see a stable UI while trimmedContent() and theme() resolve; ensure the fallback
visually matches the snippet size and replace the current empty Suspense usage
with Suspense fallback={...} referencing the same StaticMarkdown component
context.
Comment on lines
+215
to
+217
| // Kick off the lazy schema load so it is ready before the first | ||
| // notification arrives over the websocket. | ||
| void loadUnifiedNotificationSchema(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify fire-and-forget preload call has explicit rejection handling
rg -n -C2 'void loadUnifiedNotificationSchema\(\)' js/app/packages/notifications/notification-source.ts
rg -n -C2 'loadUnifiedNotificationSchema\(\)\.catch' js/app/packages/notifications/notification-source.tsRepository: macro-inc/macro
Length of output: 272
🏁 Script executed:
#!/bin/bash
# Find where loadUnifiedNotificationSchema is defined and how it handles failures.
rg -n --hidden --no-ignore -S "function loadUnifiedNotificationSchema|const loadUnifiedNotificationSchema|export (async )?function loadUnifiedNotificationSchema|loadUnifiedNotificationSchema\W*=" js/app/packages/notifications
rg -n -S "loadUnifiedNotificationSchema" js/app/packages/notifications/notification-source.tsRepository: macro-inc/macro
Length of output: 267
🏁 Script executed:
#!/bin/bash
# Show the implementation of loadUnifiedNotificationSchema and any internal error handling.
sed -n '1,120p' js/app/packages/notifications/types.tsRepository: macro-inc/macro
Length of output: 1994
🏁 Script executed:
#!/bin/bash
rg -n "loadUnifiedNotificationSchema\(\)" js/app/packages/notificationsRepository: macro-inc/macro
Length of output: 399
Handle preload failures to avoid unhandled Promise rejections
notification-source.ts fire-and-forgets void loadUnifiedNotificationSchema();, but loadUnifiedNotificationSchema() performs a dynamic import(...) and returns a promise with no internal error handling—so a failed chunk load can trigger unhandledrejection on startup.
Suggested fix
- void loadUnifiedNotificationSchema();
+ void loadUnifiedNotificationSchema().catch((error) => {
+ console.warn('Failed to load notification schema', error);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Kick off the lazy schema load so it is ready before the first | |
| // notification arrives over the websocket. | |
| void loadUnifiedNotificationSchema(); | |
| // Kick off the lazy schema load so it is ready before the first | |
| // notification arrives over the websocket. | |
| void loadUnifiedNotificationSchema().catch((error) => { | |
| console.warn('Failed to load notification schema', error); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@js/app/packages/notifications/notification-source.ts` around lines 215 - 217,
The fire-and-forget call to loadUnifiedNotificationSchema() can produce an
unhandled rejection if the dynamic import fails; change the invocation at the
top-level (the current "void loadUnifiedNotificationSchema();") to handle errors
explicitly by attaching a .catch(...) (or awaiting inside an async init) and log
or safely swallow the error. Specifically, replace the bare call with something
like: void loadUnifiedNotificationSchema().catch(err => /* use existing logger
or console.error with a clear message referencing loadUnifiedNotificationSchema
*/) so any chunk-load failures are caught and reported.
Comment on lines
+27
to
+33
| export function loadUnifiedNotificationSchema(): Promise<UnifiedNotificationSchema> { | ||
| schemaPromise ??= import('@service-notification/generated/zod').then((m) => { | ||
| loadedSchema = buildUnifiedNotificationSchema(m); | ||
| return loadedSchema; | ||
| }); | ||
| return schemaPromise; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether rejected lazy-load promises are reset for retry
rg -n -C3 'schemaPromise \?\?=' js/app/packages/notifications/types.ts
rg -n -C3 'schemaPromise\s*=\s*null' js/app/packages/notifications/types.ts
rg -n -C3 'catch\s*\(' js/app/packages/notifications/types.tsRepository: macro-inc/macro
Length of output: 401
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/notifications/types.ts"
# Show the full surrounding implementation for context
nl -ba "$FILE" | sed -n '1,120p'
# Focus around the loader and cache variables
nl -ba "$FILE" | sed -n '15,80p'
# Find any other mention of loadUnifiedNotificationSchema / schemaPromise / loadedSchema
rg -n "loadUnifiedNotificationSchema|schemaPromise|loadedSchema" "$FILE"
# Check for error handling patterns in callers (usage sites)
rg -n "loadUnifiedNotificationSchema\(" js/app/packages/notifications -SRepository: macro-inc/macro
Length of output: 102
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/notifications/types.ts"
echo "=== File excerpt (1-120) ==="
awk 'NR>=1 && NR<=120 {printf "%5d:%s\n", NR, $0}' "$FILE"
echo "=== Focus excerpt (15-60) ==="
awk 'NR>=15 && NR<=60 {printf "%5d:%s\n", NR, $0}' "$FILE"
echo "=== Loader-related symbols ==="
rg -n "loadUnifiedNotificationSchema|schemaPromise|loadedSchema" "$FILE"
echo "=== Call sites (within notifications package) ==="
rg -n "loadUnifiedNotificationSchema\\(" js/app/packages/notifications -SRepository: macro-inc/macro
Length of output: 4857
🏁 Script executed:
#!/bin/bash
set -euo pipefail
RG_TARGETS=( "js/app/packages/notifications/notification-source.ts" )
for f in "${RG_TARGETS[@]}"; do
echo "=== $f (around loadUnifiedNotificationSchema call) ==="
# print ~30 lines around the match
awk '{
if ($0 ~ /loadUnifiedNotificationSchema\(\)/) {found=NR; start=NR-20; end=NR+20}
if (NR>=start && NR<=end) {printf "%5d:%s\n", NR, $0}
}' "$f" | sed -n '1,120p'
done
echo "=== Where getUnifiedNotificationSchema() is used in notifications package ==="
rg -n "getUnifiedNotificationSchema\\(" js/app/packages/notifications -SRepository: macro-inc/macro
Length of output: 1399
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== All call sites of loadUnifiedNotificationSchema() in js/app/packages/notifications ==="
rg -n "loadUnifiedNotificationSchema\\(" js/app/packages/notifications -S
echo "=== All call sites of loadUnifiedNotificationSchema() across js/app ==="
rg -n "loadUnifiedNotificationSchema\\(" js/app -S
echo "=== Inspect notification-source.ts around the load call (190-260) ==="
FILE="js/app/packages/notifications/notification-source.ts"
awk 'NR>=180 && NR<=260 {printf "%5d:%s\n", NR, $0}' "$FILE"
echo "=== Search for any catch/then handling nearby in notification-source.ts ==="
rg -n "loadUnifiedNotificationSchema\\(|catch\\(|unhandledrejection" js/app/packages/notifications/notification-source.ts -SRepository: macro-inc/macro
Length of output: 4084
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "unhandledrejection|unhandledRejection" js/app -S
rg -n "loadUnifiedNotificationSchema\\(\\)" js/app -SRepository: macro-inc/macro
Length of output: 41
Reset the cached unified notification schema loader promise on failure (and don’t ignore the rejection).
loadUnifiedNotificationSchema() memoizes the dynamic import promise via schemaPromise ??= ... and never clears schemaPromise on rejection, so getUnifiedNotificationSchema() stays null and websocket notifications fall back to unvalidated parsing for the rest of the session. The only caller drops the promise (void loadUnifiedNotificationSchema()), so failures can also surface as unhandled rejections.
Suggested fix
export function loadUnifiedNotificationSchema(): Promise<UnifiedNotificationSchema> {
- schemaPromise ??= import('`@service-notification/generated/zod`').then((m) => {
- loadedSchema = buildUnifiedNotificationSchema(m);
- return loadedSchema;
- });
+ schemaPromise ??= import('`@service-notification/generated/zod`')
+ .then((m) => {
+ loadedSchema = buildUnifiedNotificationSchema(m);
+ return loadedSchema;
+ })
+ .catch((error) => {
+ schemaPromise = null;
+ throw error;
+ });
return schemaPromise;
}Also handle the caller’s ignored promise (e.g., attach a .catch at void loadUnifiedNotificationSchema();) so import failures don’t become unhandled rejections.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@js/app/packages/notifications/types.ts` around lines 27 - 33, The
dynamic-import memoization in loadUnifiedNotificationSchema() keeps
schemaPromise set on rejection and drops the error, so reset schemaPromise and
clear loadedSchema when the import/build fails and re-throw the error to avoid a
permanently failed cache; specifically, change the Promise chain for
schemaPromise (and buildUnifiedNotificationSchema(m) usage) to include a
.catch(err => { schemaPromise = undefined; loadedSchema = null; throw err; }) so
subsequent calls can retry, and update the caller that uses void
loadUnifiedNotificationSchema() to attach a .catch(...) to avoid unhandled
rejections.
Comment on lines
+5
to
+9
| interface User { | ||
| id: string; | ||
| email: string; | ||
| [key: string]: any; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify remaining `any` in observability sources
rg -nP '\bany\b' --type ts js/app/packages/observability/srcRepository: macro-inc/macro
Length of output: 261
Remove any from observability payload types
any still exists in observability sources ([key: string]: any) in:
js/app/packages/observability/src/index.ts:8js/app/packages/observability/src/logger.ts:6js/app/packages/observability/src/impl.ts:96
Replace these index signatures with unknown (and narrow where the payload is consumed).
Proposed type-safe adjustment
interface User {
id: string;
email: string;
- [key: string]: any;
+ [key: string]: unknown;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| interface User { | |
| id: string; | |
| email: string; | |
| [key: string]: any; | |
| } | |
| interface User { | |
| id: string; | |
| email: string; | |
| [key: string]: unknown; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@js/app/packages/observability/src/index.ts` around lines 5 - 9, Replace the
loose any index signature on the User interface and other observability payload
types with unknown and narrow at consumption sites: change "interface User { id:
string; email: string; [key: string]: any }" to use "[key: string]: unknown",
update analogous signatures in logger.ts and impl.ts to unknown, then update
code that reads these dynamic properties (e.g., inside the logger creation
functions and the implementation code around the consumer at impl.ts line ~96)
to type-check/cast or validate the expected shape before accessing fields (use
type predicates, safe property checks, or explicit casts) so payload handling
remains type-safe.
Source: Coding guidelines
Comment on lines
+157
to
+162
| function getSavedDocumentMetadataSchema(): Promise<SavedDocumentMetadataSchema> { | ||
| savedDocumentMetadataSchemaPromise ??= import('./generated/zod').then( | ||
| (m) => m.saveDocumentHandlerResponse.shape.data.shape.documentMetadata | ||
| ); | ||
| return savedDocumentMetadataSchemaPromise; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/service-clients/service-storage/client.ts"
echo "== File exists =="
ls -l "$FILE"
echo "== Declarations near getSavedDocumentMetadataSchema =="
nl -ba "$FILE" | sed -n '120,220p'
echo "== Usages around reported save callsites =="
nl -ba "$FILE" | sed -n '1420,1535p'
nl -ba "$FILE" | sed -n '1730,1860p'
echo "== All references to savedDocumentMetadataSchemaPromise =="
rg -n "savedDocumentMetadataSchemaPromise|getSavedDocumentMetadataSchema" "$FILE"
echo "== Check for try/catch around getSavedDocumentMetadataSchema callsites =="
rg -n "getSavedDocumentMetadataSchema\\(" "$FILE" -nRepository: macro-inc/macro
Length of output: 302
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/service-clients/service-storage/client.ts"
echo "== Declarations near getSavedDocumentMetadataSchema =="
# show with line numbers without relying on nl
awk 'NR>=120 && NR<=220 {printf "%d\t%s\n", NR, $0}' "$FILE"
echo "== All references to savedDocumentMetadataSchemaPromise =="
rg -n "savedDocumentMetadataSchemaPromise|getSavedDocumentMetadataSchema" "$FILE"
echo "== Usages around reported save callsites (areas 1400-1550 and 1700-1900) =="
awk 'NR>=1400 && NR<=1550 {printf "%d\t%s\n", NR, $0}' "$FILE"
awk 'NR>=1700 && NR<=1900 {printf "%d\t%s\n", NR, $0}' "$FILE"
echo "== Check for try/catch patterns near callsites =="
rg -n "try\s*\{|catch\s*\(" "$FILE"Repository: macro-inc/macro
Length of output: 16723
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/service-clients/service-storage/client.ts"
echo "== Search pdfSave/simpleSave/simpleSaveText signatures =="
rg -n "async pdfSave|async simpleSave\\(|async simpleSaveText\\(" "$FILE"
echo "== Extract around pdfSave signature area (includes return type annotations if present) =="
awk 'NR>=1425 && NR<=1528 {printf "%d\t%s\n", NR, $0}' "$FILE"
echo "== Extract around simpleSaveText signature area (includes return type annotations if present) =="
awk 'NR>=1765 && NR<=1810 {printf "%d\t%s\n", NR, $0}' "$FILE"
echo "== Check if savedDocumentMetadataSchemaPromise is ever reset to null/undefined =="
rg -n "savedDocumentMetadataSchemaPromise\s*=\s*null|savedDocumentMetadataSchemaPromise\s*=\s*undefined|savedDocumentMetadataSchemaPromise\s*=" "$FILE"Repository: macro-inc/macro
Length of output: 5166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/service-clients/service-storage/client.ts"
echo "== Find pdfSave/simpleSave/simpleSaveText in StorageServiceClient type usage =="
rg -n "pdfSave\s*|simpleSave\s*\(|simpleSaveText\s*\(" "$FILE"
echo "== Inspect around start of service client object that contains these methods =="
# find the nearest surrounding "const storageServiceClient" / "export const" / "{"
rg -n "storageServiceClient|const storage|export const|serviceClient|client\s*=" "$FILE" | head -n 20
# show a chunk earlier than pdfSave to include any return type annotations
awk 'NR>=1320 && NR<=1445 {printf "%d\t%s\n", NR, $0}' "$FILE"
# show chunk earlier than simpleSaveText to include any return type annotations
awk 'NR>=1660 && NR<=1830 {printf "%d\t%s\n", NR, $0}' "$FILE"Repository: macro-inc/macro
Length of output: 11064
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/service-clients/service-storage/client.ts"
echo "== Around storageServiceClient declaration =="
awk 'NR>=330 && NR<=430 {printf "%d\t%s\n", NR, $0}' "$FILE"
echo "== Where pdfSave/simpleSave/simpleSaveText are referenced from outside (if any) =="
rg -n "storageServiceClient\.(pdfSave|simpleSave|simpleSaveText)\b" js/app/packages -S
echo "== Locate service type file for StorageServiceClient (./service) =="
DIR="$(dirname "$FILE")"
echo "DIR=$DIR"
ls -la "$DIR"Repository: macro-inc/macro
Length of output: 4487
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/service-clients/service-storage/service.ts"
echo "== Find pdfSave/simpleSave/simpleSaveText in service.ts =="
rg -n "pdfSave|simpleSaveText|simpleSave\(" "$FILE"
echo "== Extract around the method type declarations =="
# Print a window around each match line
for line in $(rg -n "pdfSave|simpleSaveText|simpleSave\(" "$FILE" | cut -d: -f1 | sort -u); do
start=$((line-20))
end=$((line+60))
echo "--- window $start-$end around line $line ---"
awk -v s="$start" -v e="$end" 'NR>=s && NR<=e {printf "%d\t%s\n", NR, $0}' "$FILE"
done
echo "== Check ok/err type helpers location in this package =="
# look for definitions
rg -n "function ok\\b|function err\\b|const ok\\b|const err\\b|export const ok\\b|export const err\\b" js/app/packages/service-clients/service-storage -SRepository: macro-inc/macro
Length of output: 6811
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILES=(
"js/app/packages/block-pdf/signal/save.ts"
"js/app/packages/block-md/signal/save.ts"
"js/app/packages/block-canvas/store/canvasData.ts"
"js/app/packages/block-code/component/CodeMirror.tsx"
)
for f in "${FILES[@]}"; do
echo "==== $f (match windows) ===="
rg -n "storageServiceClient\\.(pdfSave|simpleSave|simpleSaveText)\\b" "$f" || true
# show surrounding code around first few matches
for line in $(rg -n "storageServiceClient\\.(pdfSave|simpleSave|simpleSaveText)\\b" "$f" | cut -d: -f1 | head -n 5); do
start=$((line-40)); end=$((line+60))
echo "--- window $start-$end around line $line ---"
awk -v s="$start" -v e="$end" 'NR>=s && NR<=e {printf "%d\t%s\n", NR, $0}' "$f"
done
doneRepository: macro-inc/macro
Length of output: 11801
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/service-clients/service-storage/service.ts"
rg -n "withFetchErrors|throws:" "$FILE"
echo "== Show withFetchErrors definition (window around its match) =="
LINE=$(rg -n "withFetchErrors" "$FILE" | head -n 1 | cut -d: -f1)
START=$((LINE-60)); END=$((LINE+80))
awk -v s="$START" -v e="$END" 'NR>=s && NR<=e {printf "%d\t%s\n", NR, $0}' "$FILE"
echo "== Show how storage client is constructed/wrapped in this file =="
rg -n "registerClient\\(|createClient\\(|createService|makeClient|service\\.client" "$FILE"Repository: macro-inc/macro
Length of output: 5737
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/service-clients/service-storage/service.ts"
# Find the tail where it registers/exports the service
echo "== Find register/export in service.ts =="
rg -n "export const|registerClient|createService|Svc\\(" "$FILE"
echo "== Show around registerClient in service.ts (if any) =="
rg -n "registerClient\\(" "$FILE" || true
if rg -n "registerClient\\(" "$FILE" >/dev/null; then
LINE=$(rg -n "registerClient\\(" "$FILE" | head -n 1 | cut -d: -f1)
awk -v s=$((LINE-80)) -v e=$((LINE+80)) 'NR>=s && NR<=e {printf "%d\t%s\n", NR, $0}' "$FILE"
fi
echo "== Find assignment of implementation to service wrapper =="
rg -n "storageServiceClient|client\\.pdfSave|impl|implementation|makeClient|getClient" "$FILE" || true
echo "== Show file end for any composition/wiring =="
tail -n 80 "$FILE"Repository: macro-inc/macro
Length of output: 3874
🏁 Script executed:
#!/bin/bash
set -euo pipefail
RGDIR="js/app/packages"
echo "== Locate withFetchErrors implementation =="
rg -n "function withFetchErrors|withFetchErrors\\s*\\(" "$RGDIR" -S
echo "== Locate core/service exports for ServiceClient/Svc =="
rg -n "export (type )?ServiceClient|class Svc|function Svc|const Svc" "$RGDIR" -S
echo "== Open likely core/service file(s) =="
fd "service.*(ts|tsx|js)" js/app/packages -a | head -n 30Repository: macro-inc/macro
Length of output: 8655
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/core/service.ts"
# show withFetchErrors implementation and nearby helper types
awk 'NR>=260 && NR<=420 {printf "%d\t%s\n", NR, $0}' "$FILE"
# show service execution wrapper around ServiceClient call behavior (look for try/catch around handler invocation)
rg -n "try\\s*\\{|catch\\s*\\(|throw|Promise\\.resolve|invoke|fn\\(" "$FILE" | head -n 80
# show around where ServiceClient methods are implemented (search for "ServiceClient" or Proxy)
rg -n "Proxy|makeProxy|client\\b|call\\b|invoke\\b|handlers\\b|mapErr|isErr" "$FILE" | head -n 80Repository: macro-inc/macro
Length of output: 6670
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/core/service.ts"
echo "== Svc class implementation window =="
awk 'NR>=120 && NR<=260 {printf "%d\t%s\n", NR, $0}' "$FILE"
echo "== Search for Proxy/call/invoke/handler in core/service.ts =="
rg -n "Proxy\\b|new Proxy\\b|invoke\\b|call\\b|handler\\b|catch\\s*\\(|try\\s*\\(|mapErr|isErr\\b|return err\\b" "$FILE"
echo "== Show around first occurrence of 'new Proxy' (if any) =="
line=$(rg -n "new Proxy" "$FILE" | head -n 1 | cut -d: -f1 || true)
if [ -n "${line:-}" ]; then
start=$((line-80)); end=$((line+120))
awk -v s=$start -v e=$end 'NR>=s && NR<=e {printf "%d\t%s\n", NR, $0}' "$FILE"
else
echo "No new Proxy found"
fi
echo "== Show around first 'catch (' occurrence =="
line=$(rg -n "catch\\s*\\(" "$FILE" | head -n 1 | cut -d: -f1 || true)
if [ -n "${line:-}" ]; then
start=$((line-80)); end=$((line+120))
awk -v s=$start -v e=$end 'NR>=s && NR<=e {printf "%d\t%s\n", NR, $0}' "$FILE"
fiRepository: macro-inc/macro
Length of output: 4920
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/core/service.ts"
echo "== Search for implementation of ServiceClient proxy/invocation =="
rg -n "new Proxy|Proxy\\s*\\(|createClient|makeClient|ServiceClient\\b|invoke\\b|handlers\\b|async \\(|fn\\(" "$FILE"
echo "== Search for any catch/try inside invocation logic =="
rg -n "catch\\s*\\(|try\\s*\\(" "$FILE"
echo "== Show later file windows around where ServiceClient types might be used (look for 'client' or 'svc' variables) =="
# show from ~420 to end
awk 'NR>=420 && NR<=620 {printf "%d\t%s\n", NR, $0}' "$FILE"
echo "== Tail of core/service.ts =="
tail -n 120 "$FILE"Repository: macro-inc/macro
Length of output: 346
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/service-clients/service-storage/client.ts"
echo "== Find registerClient call in this file =="
rg -n "registerClient\\(" "$FILE"
LINE=$(rg -n "registerClient\\(" "$FILE" | head -n 1 | cut -d: -f1)
start=$((LINE-120)); end=$((LINE+60))
echo "== Show around registerClient =="
awk -v s=$start -v e=$end 'NR>=s && NR<=e {printf "%d\t%s\n", NR, $0}' "$FILE"
echo "== Find any try/catch around pdfSave implementations =="
rg -n "try\\s*\\{|catch\\s*\\(" "$FILE" | head -n 50Repository: macro-inc/macro
Length of output: 3679
🏁 Script executed:
#!/bin/bash
set -euo pipefail
RGDIR="js/app/packages/core"
rg -n "registerClient\\b|function registerClient|export function registerClient" "$RGDIR" -S
rg -n "createClient\\b|makeClient\\b|proxy\\b|new Proxy\\b|Proxy\\b|invoke\\b|dispatch\\b|Svc\\b.*client" "$RGDIR" -S
# Also search broader for registerClient definition in app packages
rg -n "registerClient\\b" js/app/packages -S | head -n 50Repository: macro-inc/macro
Length of output: 6741
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/app/packages/core/util/mockClient.ts"
wc -l "$FILE"
sed -n '1,120p' "$FILE"Repository: macro-inc/macro
Length of output: 518
Handle document-metadata schema load failures instead of caching a rejected promise forever
getSavedDocumentMetadataSchema() memoizes the dynamic import('./generated/zod') promise in savedDocumentMetadataSchemaPromise ??= ...; if that import ever rejects once, the rejected promise is reused for all subsequent saves.
pdfSave / simpleSave / simpleSaveText then await getSavedDocumentMetadataSchema() and immediately call .safeParse(...) with no error handling, so schema-load failures aren’t converted into a recoverable Result path and can keep breaking saves until reload.
🛠️ Suggested fix pattern
let savedDocumentMetadataSchemaPromise: Promise<SavedDocumentMetadataSchema> | null =
null;
function getSavedDocumentMetadataSchema(): Promise<SavedDocumentMetadataSchema> {
- savedDocumentMetadataSchemaPromise ??= import('./generated/zod').then(
- (m) => m.saveDocumentHandlerResponse.shape.data.shape.documentMetadata
- );
+ if (!savedDocumentMetadataSchemaPromise) {
+ savedDocumentMetadataSchemaPromise = import('./generated/zod')
+ .then((m) => m.saveDocumentHandlerResponse.shape.data.shape.documentMetadata)
+ .catch((error) => {
+ savedDocumentMetadataSchemaPromise = null; // allow retry
+ throw error;
+ });
+ }
return savedDocumentMetadataSchemaPromise;
}-const metadata = (await getSavedDocumentMetadataSchema()).safeParse(data.documentMetadata);
+let schema: SavedDocumentMetadataSchema;
+try {
+ schema = await getSavedDocumentMetadataSchema();
+} catch {
+ return err([
+ {
+ code: 'INVALID_RESPONSE',
+ message: 'Failed to load document metadata schema',
+ },
+ ]);
+}
+const metadata = schema.safeParse(data.documentMetadata);Apply to pdfSave (1482-1484), simpleSave (1511-1513), and simpleSaveText (1792-1794).
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @js/app/packages/service-clients/service-storage/client.ts around lines 157 -
162, getSavedDocumentMetadataSchema currently memoizes a failing dynamic import
forever; change it to catch errors from import('./generated/zod'), reset
savedDocumentMetadataSchemaPromise to undefined (so a transient failure doesn't
get cached) and rethrow the error so callers can handle it; then update pdfSave,
simpleSave, and simpleSaveText to wrap the await
getSavedDocumentMetadataSchema() in a try/catch and convert import/schema-load
errors into the recoverable Result failure path before calling .safeParse (or
bail early), ensuring you reference savedDocumentMetadataSchemaPromise,
getSavedDocumentMetadataSchema, and the three save functions when making the
changes.
</details>
<!-- fingerprinting:phantom:poseidon:hawk -->
<!-- cr-comment:v1:194080a3b455d483b8e5ed98 -->
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
113
to
142
| handleAccept(editor: LexicalEditor) { | ||
| editor.update(() => { | ||
| const diffInsertNode = this.getChildren().find((child) => | ||
| $isDiffInsertNode(child) | ||
| ); | ||
| if (!diffInsertNode) return; | ||
|
|
||
| const insertMarkdown = diffInsertNode.getMarkdown(); | ||
| if (insertMarkdown === undefined) return; | ||
|
|
||
| this.clear(); | ||
|
|
||
| $convertFromMarkdownString(insertMarkdown, ALL_TRANSFORMERS, this, false); | ||
|
|
||
| const lastChild = this.getLastChild(); | ||
| $unwrapNode(this); | ||
| if (lastChild) { | ||
| lastChild.selectEnd(); | ||
| } | ||
| // The markdown conversion stack loads lazily: this node class is | ||
| // registered at app boot, and importing @lexical/markdown + the | ||
| // transformer list eagerly would pull them into the initial bundle. | ||
| // handleAccept is a user action (accepting a diff), so deferring the | ||
| // update until the one-time chunk load resolves is fine. | ||
| void Promise.all([ | ||
| import('@lexical/markdown'), | ||
| import('../transformers'), | ||
| ]).then(([{ $convertFromMarkdownString }, { ALL_TRANSFORMERS }]) => { | ||
| editor.update(() => { | ||
| const diffInsertNode = this.getChildren().find((child) => | ||
| $isDiffInsertNode(child) | ||
| ); | ||
| if (!diffInsertNode) return; | ||
|
|
||
| const insertMarkdown = diffInsertNode.getMarkdown(); | ||
| if (insertMarkdown === undefined) return; | ||
|
|
||
| this.clear(); | ||
|
|
||
| $convertFromMarkdownString(insertMarkdown, ALL_TRANSFORMERS, this, false); | ||
|
|
||
| const lastChild = this.getLastChild(); | ||
| $unwrapNode(this); | ||
| if (lastChild) { | ||
| lastChild.selectEnd(); | ||
| } | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file
ls -la js/lexical-core/nodes/DiffNode.ts
# Show handleAccept implementation with line numbers
sed -n '1,220p' js/lexical-core/nodes/DiffNode.ts | nl -ba | sed -n '80,220p'
# Show full file if it's short, else show remainder around handleAccept end
wc -l js/lexical-core/nodes/DiffNode.ts
sed -n '220,420p' js/lexical-core/nodes/DiffNode.ts | nl -ba
# Search for handleAccept usage
rg -n "handleAccept\\(" -S js/lexical-core/nodes/DiffNode.ts js/lexical-core -S || true
# Search for similar lazy import + editor.update + catch patterns
rg -n "Promise\\.all\\(\\[\\s*import\\(" js -S | head -n 50 || true
rg -n "\\.catch\\(" js/lexical-core/nodes js/lexical-core -S | head -n 50 || true
rg -n "lazy-load markdown" -S js | head -n 50 || trueRepository: macro-inc/macro
Length of output: 203
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file
ls -la js/lexical-core/nodes/DiffNode.ts
# Show handleAccept implementation with line numbers
sed -n '1,260p' js/lexical-core/nodes/DiffNode.ts | nl -ba | sed -n '80,260p'
# Show file length and tail around if needed
wc -l js/lexical-core/nodes/DiffNode.ts
tail -n 80 js/lexical-core/nodes/DiffNode.ts | nl -ba
# Search for handleAccept usage
rg -n "handleAccept\\(" -S js/lexical-core | head -n 50 || true
# Search for similar lazy import patterns
rg -n "Promise\\.all\\(\\[\\s*import\\(" js/lexical-core -S | head -n 50 || true
rg -n "\\.catch\\(" js/lexical-core -S | head -n 50 || true
# Find other dynamic import usage without catch
rg -n "void Promise\\.all\\(" js/lexical-core -S || true
rg -n "import\\([^\\)]*\\)\\)\\.then\\(" js/lexical-core -S | head -n 50 || trueRepository: macro-inc/macro
Length of output: 203
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la js/lexical-core/nodes/DiffNode.ts
sed -n '1,260p' js/lexical-core/nodes/DiffNode.ts | nl -ba | sed -n '90,200p'
wc -l js/lexical-core/nodes/DiffNode.ts
sed -n '200,320p' js/lexical-core/nodes/DiffNode.ts | nl -ba
rg -n "handleAccept\\(" -S js/lexical-core | head -n 50 || true
rg -n "Promise\\.all\\(\\[\\s*import\\(" js/lexical-core -S | head -n 50 || true
rg -n "\\.catch\\(" js/lexical-core -S | head -n 50 || true
rg -n "import\\([^\\)]*\\)\\)\\.then\\(" js/lexical-core -S | head -n 50 || trueRepository: macro-inc/macro
Length of output: 203
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la js/lexical-core/nodes/DiffNode.ts
sed -n '1,260p' js/lexical-core/nodes/DiffNode.ts | nl -ba | sed -n '90,200p'
wc -l js/lexical-core/nodes/DiffNode.ts
sed -n '200,320p' js/lexical-core/nodes/DiffNode.ts | nl -ba
rg -n "handleAccept\\(" -S js/lexical-core | head -n 50 || true
rg -n "Promise\\.all\\(\\[\\s*import\\(" js/lexical-core -S | head -n 50 || true
rg -n "\\.catch\\(" js/lexical-core -S | head -n 50 || true
rg -n "import\\([^\\)]*\\)\\)\\.then\\(" js/lexical-core -S | head -n 50 || trueRepository: macro-inc/macro
Length of output: 203
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la js/lexical-core/nodes/DiffNode.ts
sed -n '1,260p' js/lexical-core/nodes/DiffNode.ts | nl -ba | sed -n '90,200p'
wc -l js/lexical-core/nodes/DiffNode.ts
sed -n '200,320p' js/lexical-core/nodes/DiffNode.ts | nl -ba
rg -n "handleAccept\\(" -S js/lexical-core | head -n 50 || true
rg -n "Promise\\.all\\(\\[\\s*import\\(" js/lexical-core -S | head -n 50 || true
rg -n "\\.catch\\(" js/lexical-core -S | head -n 50 || true
rg -n "import\\([^\\)]*\\)\\)\\.then\\(" js/lexical-core -S | head -n 50 || trueRepository: macro-inc/macro
Length of output: 203
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '90,170p' js/lexical-core/nodes/DiffNode.ts | nl -ba
wc -l js/lexical-core/nodes/DiffNode.ts
rg -n "handleAccept\\(" js/lexical-core/nodes/DiffNode.ts js/lexical-core -S | head -n 20 || true
rg -n "\\.catch\\(" js/lexical-core/nodes/DiffNode.ts js/lexical-core -S | head -n 20 || trueRepository: macro-inc/macro
Length of output: 102
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "FILE:"
ls -la js/lexical-core/nodes/DiffNode.ts
echo "SNIPPET:"
nl -ba js/lexical-core/nodes/DiffNode.ts | sed -n '90,170p'
echo "MATCHES:"
rg -n "handleAccept\\(" -S js/lexical-core/nodes/DiffNode.tsRepository: macro-inc/macro
Length of output: 218
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find DiffNode.ts
fd "DiffNode.ts" -a . | head -n 20Repository: macro-inc/macro
Length of output: 112
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="js/lexical-core/nodes/DiffNode.ts"
echo "handleAccept matches:"
rg -n "handleAccept\\(" "$FILE" || true
echo
echo "Promise/all + dynamic imports in file:"
rg -n "Promise\\.all\\(|import\\(" "$FILE" || true
echo
echo "catch clauses in file:"
rg -n "\\.catch\\(" "$FILE" || true
echo
echo "Printing lines ~90-170 (no nl needed):"
sed -n '90,170p' "$FILE"
echo
echo "Printing lines ~170-260 (no nl needed):"
sed -n '170,260p' "$FILE"
echo
echo "Line-numbered excerpt of handleAccept block (Python):"
python3 - <<'PY'
import re
path="js/lexical-core/nodes/DiffNode.ts"
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
for i,l in enumerate(lines, start=1):
if "handleAccept" in l:
start=i-5
end=i+120
for j in range(max(1,start), min(len(lines), end)+1):
print(f"{j:4d}: {lines[j-1].rstrip()}")
break
else:
print("No handleAccept found")
PYRepository: macro-inc/macro
Length of output: 8414
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "handleAccept\\(" -S js | head -n 50 || true
rg -n "handleReject\\(" -S js | head -n 50 || true
# Find DiffNode.accept call sites more generally
rg -n "\\.handleAccept\\b" -S js | head -n 50 || true
# Search for patterns: dynamic imports + catch in lexical-core
rg -n "import\\([^\\)]*\\)\\s*\\)\\s*\\.catch\\(" -S js/lexical-core | head -n 50 || true
rg -n "\\.catch\\(" -S js/lexical-core | head -n 50 || true
# Search for use of $getNodeByKey in this repo
rg -n "\\$getNodeByKey\\(" -S js | head -n 50 || trueRepository: macro-inc/macro
Length of output: 650
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Call sites for handleAccept:"
rg -n "\\.handleAccept\\b" -S js | head -n 50 || true
echo
echo "Call sites for DiffNode.handleAccept:"
rg -n "handleAccept\\(" -S js | head -n 50 || true
echo
echo "Dynamic import + catch patterns (lexical-core):"
rg -n "import\\([^\\)]*\\)\\s*\\)\\s*\\.catch\\(" -S js/lexical-core | head -n 50 || true
echo
echo "Catch clauses (lexical-core, limited):"
rg -n "\\.catch\\(" -S js/lexical-core | head -n 50 || true
echo
echo "Usages of $getNodeByKey:"
rg -n "\\$getNodeByKey\\(" -S js | head -n 50 || trueRepository: macro-inc/macro
Length of output: 608
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Call sites for handleAccept:"
rg -n "\\.handleAccept\\b" -S js | head -n 50 || true
echo
echo "Call sites for handleAccept( (any):"
rg -n "handleAccept\\(" -S js | head -n 50 || true
echo
echo "Dynamic import + catch patterns (lexical-core):"
rg -n "import\\([^\\)]*\\)\\s*\\)\\s*\\.catch\\(" -S js/lexical-core | head -n 50 || true
echo
echo "Catch clauses (lexical-core, limited):"
rg -n "\\.catch\\(" -S js/lexical-core | head -n 50 || true
echo
echo "Usages of $getNodeByKey:"
rg -n "\\$getNodeByKey\\(" -S js | head -n 50 || trueRepository: macro-inc/macro
Length of output: 606
Make DiffNode.handleAccept async-safe (captured node + missing catch)
- The lazy
Promise.all(...).then(...)still operates on the capturedthisnode insideeditor.update(...); re-resolve the node insideeditor.update(e.g., via key) before callinggetChildren(),clear(),$convertFromMarkdownString(..., this, ...), and$unwrapNode(this). - Add a
.catch(...)to the lazy import chain to avoid unhandled promise rejections if the dynamic imports fail.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@js/lexical-core/nodes/DiffNode.ts` around lines 113 - 142, Capture the node
key before the lazy imports and re-resolve the node inside editor.update to
avoid operating on a possibly-stale/captured `this` (e.g., in
DiffNode.handleAccept do const nodeKey = this.getKey(); then inside
editor.update call const node = $getNodeByKey(nodeKey) and bail if null before
calling node.getChildren(), node.clear(), $convertFromMarkdownString(..., node,
...), and $unwrapNode(node)); also append a .catch(...) to the Promise.all(...)
import chain to handle and log import errors so you don't leave unhandled
promise rejections.
816e45b to
f91a575
Compare
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
2 participants
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.
This PR significantly reduces the initial bundle size by deferring the loading of heavy dependencies until they're actually needed. The changes follow a consistent pattern: move SDK initialization code into separate modules that are dynamically imported, and provide lightweight synchronous accessors for code that runs before the SDKs load.
Key Changes
Observability (Datadog)
impl.tsmoduleinit()inindex.tsnow dynamically imports and initializes the SDKs asynchronouslylogger.ts,actionTracker.ts, androutingTracker.tsto use lazy accessorsLiveKit & Krisp
livekit-loader.tswith lazy loaders forlivekit-clientand@livekit/krisp-noise-filtergetLivekit(),getKrisp()) and async loaders (loadLivekit(),loadKrisp())LK_CONNECTION_STATE,LK_TRACK_SOURCE) so call state can be tracked without loading the SDKCallContext.tsxto use the loaders and enum mirrors instead of direct importsLexical/Markdown Editor
plugins/commands.tsto keep them out of the boot pathcontext/wrapperContext.tsso decorators don't pull plugin machineryinit.tsto import node classes individually instead of from the barrel exportEmojiMenu,InteractiveOnboarding,ShareModal,StaticMarkdownPostHog Analytics
capture,identify,reset,onFeatureFlags)Other Optimizations
block-pdf/constants.tsto avoid pulling the viewer into light consumersshareButtonRefetch.tsandshareDrawer.tsto keep share modal UI out of initial bundleautomationComposerState.tsto separate state from UI componentsImplementation Details
import()with promise caching to avoid duplicate loadsnulluntil the async load completes, allowing graceful fallbackshttps://claude.ai/code/session_01B2vhy7mk22vaiTvzEKPVhw