feat: async import loader for JS via static preload (#476)#820
Open
stephenamar-db wants to merge 5 commits intomasterfrom
Open
feat: async import loader for JS via static preload (#476)#820stephenamar-db wants to merge 5 commits intomasterfrom
stephenamar-db wants to merge 5 commits intomasterfrom
Conversation
Collaborator
Author
Adds Preloader + ImportFinder to discover imports from each file's AST and drive caller-controlled (async) loading before synchronous evaluation. Exposes SjsonnetMain.interpretAsync for the JS build, accepting a Promise-returning loader so callers can use fetch / FileReader / etc. Co-authored-by: Isaac
stephenamar-db
force-pushed
the
lazyimporter
branch
from
4c3fb08 to
6758e7a
Compare
Scala 2.13/2.12 promote "private default argument never used" to an error; the binary-loader and log defaults in the test helpers were unused at every call site. Inline them since no test exercises binary imports through the async path. Co-authored-by: Isaac
He-Pin
reviewed
May 2, 2026
Contributor
Three correctness issues with interpretAsync:
1. Preloader now passes the importing file's parent directory as
docBase to the resolver, matching Importer.resolveAndReadOrFail.
Before, a JS resolver doing `wd + "/" + importName` would look up
nested imports under `dir/a.libsonnet/b.libsonnet` instead of
`dir/b.libsonnet`.
2. interpretAsync now feeds extVar/tlaVar snippets through the
preloader so any imports they contain land in the cache. Without
this, `std.extVar('cfg')` where `cfg` is `import 'lib.libsonnet'`
failed at evaluation time against the cache-only importer.
3. Parse errors on discovered (non-entry) files no longer abort
preload. Jsonnet evaluation is lazy, so a parse error in
`if false then import 'bad' else 1` should not fail evaluation;
the interpreter surfaces the error only if the branch is forced.
Matches jrsonnet's preloader behavior.
Co-authored-by: Isaac
…loading Adds an optional preParsedAst hook on ResolvedFile and a PreParsedResolvedFile wrapper. CachedResolver.parse uses the attached AST when present, skipping fastparse and going straight to the static optimizer. Preloader now stashes the AST it produced during discovery on the cache entry, so each file is fastparse'd once total instead of twice (once for discovery, once for evaluation). Also expands the interpretAsync and Preloader docstrings to make the eager front-loading semantics explicit: every reachable import is loaded up front, including ones inside branches the evaluator will never force; this is the tradeoff that makes synchronous evaluation possible. Loader rejection still propagates; parse errors on discovered (non-entry) files are tolerated. Co-authored-by: Isaac
He-Pin
reviewed
May 7, 2026
|
|
||
| private def parseStringMap(label: String, value: js.Any): Map[String, String] = | ||
| try { | ||
| ujson.WebJson.transform(value, ujson.Value).obj.toMap.map { |
Contributor
There was a problem hiding this comment.
seems this will generate much allocation
He-Pin
reviewed
May 7, 2026
He-Pin
reviewed
May 7, 2026
He-Pin
reviewed
May 7, 2026
He-Pin
suggested changes
May 7, 2026
Contributor
He-Pin
left a comment
He-Pin
left a comment
There was a problem hiding this comment.
Found two async correctness issues and left inline comments: mixed import kinds can overwrite the preload cache for the same resolved path, and entry parse errors bypass the normal interpreter error formatting.
Four issues from the review pass:
1. parseStringMap allocations: stop round-tripping through ujson —
iterate the JS dictionary directly into a Map.newBuilder. Drops the
intermediate ujson tree, the .obj.toMap copy, and the trailing .map.
2. Naming: rename ImportFinder.Kind to a top-level ImportKind, matching
the existing ExternalVariableKind convention in the codebase.
3. Cache key collision for importstr/importbin of the same path. The
cache was keyed by Path only, so a program like
`if true then importstr "x" else importbin "x"` had whichever load
ran last clobber the other; sync interpret returns text, async was
rejecting with NotImplementedError. Cache is now keyed by
(Path, binaryData), pending dedup tracks the same key, and a new
record() helper upgrades a pending Str entry to Code if a later
reference needs the AST. Added putContent that refuses to downgrade
a PreParsedResolvedFile back to plain text.
4. Entry parse error formatting: interpretAsync was rejecting with
err.getMessage, bypassing Error.formatError used by interpret0. Now
we let the entry's parse error fall through to runInterpret so the
message has the same shape (root frame, "(memory):line:col") as
synchronous interpret.
New tests:
- PreloaderTests.scala: importstr+importbin for the same path keep
separate cache entries.
- InterpretAsyncTests.scala: same case end-to-end through the JS API,
and a test that the entry parse error message contains "(memory)".
Co-authored-by: Isaac
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
3 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.
Adds Preloader + ImportFinder to discover imports from each file's AST and drive caller-controlled (async) loading before synchronous evaluation. Exposes SjsonnetMain.interpretAsync for the JS build, accepting a Promise-returning loader so callers can use fetch / FileReader / etc.
Co-authored-by: Isaac