Skip to content

interpreter: load data files with an explicit ?format= override - #1429

Open
daandemeyer wants to merge 1 commit into
facebook:mainfrom
daandemeyer:push-lnvwyyuvyxrr
Open

interpreter: load data files with an explicit ?format= override#1429
daandemeyer wants to merge 1 commit into
facebook:mainfrom
daandemeyer:push-lnvwyyuvyxrr

Conversation

@daandemeyer

Copy link
Copy Markdown
Contributor

Loading static data with load() required the file to be named .json or
.toml, because the extension was the only thing selecting a parser. That rules
out the files people actually want to read: Cargo.lock is TOML, a
package.json.tmpl is JSON, and neither can be renamed.

Add a ?format= query on the load path (load(":deps.lock?format=toml", ...))
accepting bzl, json, or toml. It cannot be a load() kwarg: format = "toml" is already valid syntax meaning "alias the local name format to the
exported symbol toml", so taking that spelling would need a grammar change and
would silently change the meaning of existing code. The ? was already reserved
by ImportPath.

The format is stored on ImportPath and participates in its hash and equality,
so it is part of the DICE key rather than a side channel. That makes the loaded
file a tracked dependency of the parse and gets invalidation for free, and the
extension check is skipped exactly when a format was given. Every site that
dispatched on the extension now asks ImportPath::load_format() instead.

A format that merely restates the extension collapses to None, so
:a.bzl?format=bzl and :a.bzl stay one module. Otherwise the same file would
be evaluated under two keys and hand out two copies of its providers and
transitive sets, breaking the pointer-identity checks that resolve_load
already warns about.

An unparseable file stays a load-time error, which fails the whole package.
That is inherent: a parse that is not at load time is not a tracked dependency.
Since the extension no longer implies the parser, the error now names the format
and where it came from. The blast radius is documented, with a pointer to
reading the file in an action when failure needs to be scoped to its consumers.

Incidentally fixes prelude .toml files being evaluated as Starlark: the
prelude branch special-cased only .json, and now shares the format dispatch.

Covered by unit tests for format inference, the relaxed extension check, the
redundant-format collapse and display round-tripping, plus e2e tests for the
three formats, invalidation on edit, and each error path.

Signed-off-by: Daan De Meyer daan@amutable.com

Loading static data with `load()` required the file to be named `.json` or
`.toml`, because the extension was the only thing selecting a parser. That rules
out the files people actually want to read: `Cargo.lock` is TOML, a
`package.json.tmpl` is JSON, and neither can be renamed.

Add a `?format=` query on the load path (`load(":deps.lock?format=toml", ...)`)
accepting `bzl`, `json`, or `toml`. It cannot be a `load()` kwarg: `format =
"toml"` is already valid syntax meaning "alias the local name `format` to the
exported symbol `toml`", so taking that spelling would need a grammar change and
would silently change the meaning of existing code. The `?` was already reserved
by `ImportPath`.

The format is stored on `ImportPath` and participates in its hash and equality,
so it is part of the DICE key rather than a side channel. That makes the loaded
file a tracked dependency of the parse and gets invalidation for free, and the
extension check is skipped exactly when a format was given. Every site that
dispatched on the extension now asks `ImportPath::load_format()` instead.

A format that merely restates the extension collapses to `None`, so
`:a.bzl?format=bzl` and `:a.bzl` stay one module. Otherwise the same file would
be evaluated under two keys and hand out two copies of its providers and
transitive sets, breaking the pointer-identity checks that `resolve_load`
already warns about.

An unparseable file stays a load-time error, which fails the whole package.
That is inherent: a parse that is not at load time is not a tracked dependency.
Since the extension no longer implies the parser, the error now names the format
and where it came from. The blast radius is documented, with a pointer to
reading the file in an action when failure needs to be scoped to its consumers.

Incidentally fixes prelude `.toml` files being evaluated as Starlark: the
prelude branch special-cased only `.json`, and now shares the format dispatch.

Covered by unit tests for format inference, the relaxed extension check, the
redundant-format collapse and display round-tripping, plus e2e tests for the
three formats, invalidation on edit, and each error path.

Signed-off-by: Daan De Meyer <daan@amutable.com>
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 31, 2026
@meta-codesync

meta-codesync Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

This pull request has been imported. If you are a Meta employee, you can view this in D114348139. (Because this pull request was imported automatically, there will not be any future comments.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant