fix(appkit): lazy-load @ast-grep/napi so a missing native binary degrades gracefully#457
Open
atilafassina wants to merge 1 commit into
Open
fix(appkit): lazy-load @ast-grep/napi so a missing native binary degrades gracefully#457atilafassina wants to merge 1 commit into
atilafassina wants to merge 1 commit into
Conversation
…ades gracefully @ast-grep/napi's platform binary ships as an optionalDependency. When it is omitted (e.g. a Databricks Apps remote-install running `npm install --omit=optional`), the parent package throws MODULE_NOT_FOUND on require, so a top-level import crashed the appkit CLI, the @databricks/appkit barrel, and typegen with an uncaught stack. Load @ast-grep/napi lazily via createRequire behind two memoized loaders (shared CLI + appkit) so it is never touched at module-eval time. ast-grep is optional in some paths (serving-endpoint extraction, the dev source-loc Vite plugin), which now degrade to existing fallbacks with a warning; the commands that require it (lint, plugin sync, codemod) surface a clear actionable error and exit non-zero instead of a raw MODULE_NOT_FOUND. Also narrow the generate-types missing-module catch so a native-binary failure is not misreported as a missing @databricks/appkit. Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
Contributor
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes @ast-grep/napi load lazily (instead of at module-eval time) so environments missing the platform native binary degrade gracefully: appkit features skip with warnings, and CLI commands emit actionable messages instead of crashing with MODULE_NOT_FOUND.
Changes:
- Introduces lazy, memoized ast-grep loaders for appkit runtime code and the shared CLI.
- Routes ast-grep call sites (serving endpoint extractor, React source-location Vite plugin, CLI lint/codemod/plugin sync) through the loaders with graceful fallback / clearer error output.
- Adds “degrade” test coverage for the unavailable-binary paths and tightens generate-types’ missing-appkit detection.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/cli/commands/plugin/sync/sync.ts | Switches server-file parsing to lazy ast-grep loading; improves error printing for missing binary. |
| packages/shared/src/cli/commands/plugin/sync/sync.test.ts | Loads ast-grep via the lazy loader to mirror command behavior. |
| packages/shared/src/cli/commands/lint.ts | Loads ast-grep lazily and exits with an actionable message when unavailable. |
| packages/shared/src/cli/commands/generate-types.ts | Avoids misreporting unrelated missing-module errors as “appkit not installed”. |
| packages/shared/src/cli/commands/codemod/on-plugins-ready.ts | Loads ast-grep lazily and exits cleanly on unavailable-binary errors. |
| packages/shared/src/cli/commands/codemod/tests/on-plugins-ready.degrade.test.ts | Adds coverage asserting unavailable-binary errors are wrapped/typed correctly. |
| packages/shared/src/cli/ast-grep.ts | Adds shared CLI lazy loader + typed error for required ast-grep usage. |
| packages/shared/src/cli/ast-grep.test.ts | Adds tests for lazy-loader behavior when the native binary is missing. |
| packages/appkit/src/type-generator/serving/server-file-extractor.ts | Degrades serving-endpoint auto-discovery when ast-grep is unavailable. |
| packages/appkit/src/type-generator/serving/tests/server-file-extractor.test.ts | Warms the lazy-loader cache to avoid fs-mocking interference. |
| packages/appkit/src/type-generator/serving/tests/server-file-extractor.degrade.test.ts | Adds coverage for graceful degrade + warning when ast-grep is unavailable. |
| packages/appkit/src/plugins/server/react-source-loc-vite-plugin.ts | Degrades dev-only source-location annotation when ast-grep is unavailable; warns once. |
| packages/appkit/src/plugins/server/tests/react-source-loc-vite-plugin.degrade.test.ts | Adds coverage for the warn-once degrade behavior. |
| packages/appkit/src/internal/ast-grep.ts | Adds appkit-side lazy loader used by optional runtime conveniences. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+47
to
+51
| try { | ||
| mod = _require("@ast-grep/napi"); | ||
| } catch { | ||
| mod = null; | ||
| } |
Comment on lines
+32
to
+36
| try { | ||
| mod = _require("@ast-grep/napi"); | ||
| } catch { | ||
| mod = null; | ||
| } |
Comment on lines
100
to
104
| if ( | ||
| error instanceof Error && | ||
| error.message.includes("Cannot find module") | ||
| error.message.includes("Cannot find module") && | ||
| error.message.includes("@databricks/appkit") | ||
| ) { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Lazy-loads
@ast-grep/napibehind guarded loaders (packages/appkit/src/internal/ast-grep.ts,packages/shared/src/cli/ast-grep.ts) and routes every previously-eager call site through them: the serving endpoint extractor, the React source-location Vite plugin, and the shared CLI commands (lint,codemod/on-plugins-ready,plugin/sync,generate-types).Why
On Databricks Apps deploy,
postinstall → appkit generate-typeseagerly loads@ast-grep/napi. When the platform-native binary (@ast-grep/napi-linux-x64-gnu) is absent — npm silently skips the listed optional sibling via npm/cli#4828 (cross-platformpackage-lock.json) or a supply-chain cutoff — the nativerequirethrowsMODULE_NOT_FOUNDand the deploy crashes. The SDK barrel also statically re-exports the importer, so the binding was loaded at server boot too.This makes a missing binary survivable: ast-grep is loaded on demand inside a guard, and the affected commands degrade gracefully (warn + skip) instead of crashing. Nothing at server runtime needs ast-grep (build/dev-time only), so this also removes the boot-time coupling.
Scope
This is the survivability layer only. It is not the root-cause fix — ensuring the binary is actually present (so serving-type generation isn't silently skipped) is a stacked follow-up (
napi-postinstallin the published package). Tracked in the internal PRD/tasks.Verification
build/-r typecheck/test/checkgreen on this branch merged onto currentmain..degradetests for the serving extractor, the Vite plugin, the shared ast-grep loader, and the plugin codemod.Refs: npm/cli#4828.
This pull request and its description were written by Isaac.