fix: Restore lazy type resolution during load#2157
Conversation
|
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 There is also a real ergonomics/correctness argument for the current eager behavior. Eager feature resolution is enough for properties such as 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. |
Restores pre-editions lazy type resolution for
Root.fromJSON(),Root.load(), andRoot.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 callresolveAlleagerly, in turn introducing performance regressions. It has since been attempted to mitigate some of the fallout, but to me it appears that eagerly callingresolveAllis 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.