Repository navigation
Conversation
… extends TSConfigJSON::experimental_decorators and emit_decorator_metadata are tri-state. The extends merge lives in TSConfigJSON::apply_extended_config, which names every field, so the config that extends another replaces the options it sets and inherits the rest. The sema twin ParsedConfig::inherit does the same.
Collaborator
Author
This branch has not been deployed
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.
Fixes #48558
Problem
experimentalDecorators: truein atsconfig.jsonthat has anextendsline is ignored when the base file does not set it.bun a.tsthen lowers@legacy x = 1as a TC39 field decorator (the decorator gets 2 arguments instead of 3). MobX reportsPlease use @observable accessorfor such projects.extendsmerge loop inResolver::load_tsconfig(src/resolver/resolver.rs:6845). It copiesemitDecoratorMetadata,useDefineForClassFields,baseUrl,jsxandpathsfrom the extending file, and neverexperimentalDecorators. The flag was added toTSConfigJSONin feat(transpiler): implement TC39 standard ES decorators lowering #26436 and the loop was not updated.bool, so an extending file cannot turn off a flag its base turns on.tscapplies eachcompilerOptionskey of the extending file over the base.Fix
TSConfigJSON::emit_decorator_metadataandexperimental_decoratorsareOption<bool>: unset, or the value the file writes. Readers callemit_decorator_metadata()andexperimental_decorators(), which returnfalsewhen unset.TSConfigJSON::apply_extended_config(&mut self, child). It destructures every field ofchildwithout.., so a field added to the struct does not compile until the merge says how it crossesextends. A value the extending file sets replaces the value of the base (child.or(base)), as foruseDefineForClassFieldstoday.ParsedConfig::inheritinsrc/sema/resolve/code/config.rs, the copy of this merge thatbun checkand--aotuse, gets the same tri-state fields and the same rule. Its tests that pinned the old behaviour are rewritten for the new one.test/js/bun/resolve/tsconfig-extends-leak.test.ts(10 new tests, 5 fail on bun 1.4.3). Alsocargo test -p bun_resolver -- tsconfig_json,cargo test -p bun_sema -- resolve::code::config, and the decorator,useDefineForClassFields,tsconfig-overrideand aot tsconfig suites undertest/bundler/transpiler,test/cli/runandtest/aot.Background
load_tsconfigwalks theextendschain to its base, then folds each extending file onto the base, nearest file last. The foldedTSConfigJSONis stored once per directory and read on every resolve, so the merge runs once per project and the readers pay nothing new.bun checkandbun run --aotread tsconfig throughbun_sema, which has its ownParsedConfigand its owninherit. That copy mirrored the old runtime behaviour on purpose (a comment said so), so it changes in the same PR.mc.experimental_decorators ||= child.experimental_decorators, likeemitDecoratorMetadata): it fixes the reported case and keepsfalsein the extending file from clearingtruein the base, for both flags. typescript: match tsc for experimentalDecorators and emitDecoratorMetadata #46553 (a larger, open rewrite) moves to the sameOption<bool>and.or()shape as this PR, so the two do not disagree on semantics.extendswork (array form, package specifiers). This PR is the fix on its own.Downsides
experimentalDecorators: falseoremitDecoratorMetadata: falsein an extending file over a base withtruenow gets the value of the extending file, as undertsc. Before,truewon foremitDecoratorMetadata, and the base's value forexperimentalDecorators.size_of::<TSConfigJSON>()is 312 bytes before and after (Option<bool>uses the niche). The merge runs once per directory that has atsconfig.jsonwithextends. For a 3-deep chain the debug allocation log shows 3new(TSConfigJSON)and 2destroy(TSConfigJSON), the same counts as before. Release binary size not measured:bloatyis not installed here.straceis not installed here, so this is from the code, not a count.Notes
test.eachblock of the new tests.abs_pathof the extending file, the file the directory has. Only a resolver debug note reads it.preserve_imports_not_used_as_valueskeeps its merge rule. Its default isSome(false), so the extending file always wins. Nothing outside the resolver reads it.test/cli/check/check.test.tsandtest/cli/run/run-aot.test.tswere run too. Their failures in this container are timeouts at 5 s under the ASAN build when run beside another file, and one--aotstack trace case that lacks arequire (native)frame. That case reads no tsconfig option. The tsconfig cases of both files pass alone.