Skip to content

fix: Restore lazy type resolution during load#2157

Closed
dcodeIO wants to merge 1 commit intomasterfrom
patch/restore-lazy
Closed

fix: Restore lazy type resolution during load#2157
dcodeIO wants to merge 1 commit intomasterfrom
patch/restore-lazy

Conversation

@dcodeIO
Copy link
Copy Markdown
Member

@dcodeIO dcodeIO commented Apr 24, 2026

Restores pre-editions lazy type resolution for Root.fromJSON(), Root.load(), and Root.loadSync() while still resolving editions features after loading.

The context here is that before editions, we did not eagerly call resolveAll. During work on editions, which required feature resolution, this was changed to call resolveAll eagerly, in turn introducing performance regressions. It has since been attempted to mitigate some of the fallout, but to me it appears that eagerly calling resolveAll is the root cause here. Hence this PR aims at bringing us back to pre-editions baseline by resolving only features eagerly but defer type resolution as usual.

This is technically a breaking change for anyone already relying on eager type resolution after load. As far as I can tell, that behavior was not documented, and callers that need full validation can still call resolveAll() explicitly.

Related to #2123.

Follow-up to #2063 and #2066.
Partially reverts the eager load-time type resolution introduced by #2068.

@dcodeIO dcodeIO marked this pull request as draft April 30, 2026 13:50
@dcodeIO
Copy link
Copy Markdown
Member Author

dcodeIO commented Apr 30, 2026

After looking into this again, I think the tradeoff is more nuanced than my original PR description suggests.

The performance argument for this change is weaker than I initially assumed. On current master, the fully resolved path appears roughly unchanged: callers doing Root.fromJSON(...).resolveAll() pay about the same total cost with or without this PR. The measurable win is limited to callers that only need the loaded reflection tree with editions features resolved, but do not need full type-reference resolution.

There is also a real ergonomics/correctness argument for the current eager behavior. Eager feature resolution is enough for properties such as required, but type-dependent properties still needs full resolution. For example, delimited is not fully initialized until fields are resolved. This means the current eager behavior makes load/fromJSON return a more consistently usable reflection tree.

Given that, I am no longer confident that restoring lazy type resolution is the better default.

Moved this PR back to draft for now. Additional context from the editions work would still be useful, especially around whether future editions work depends on eager type resolution rather than eager feature resolution alone.

Also cc @alexander-fenster, in case you have followed the editions work more closely than I have.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant