-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor node factory API, use node factory in parser #35282
Conversation
@typescript-bot perf test |
Heya @rbuckton, I've started to run the perf test suite on this PR at 922ab41. You can monitor the build here. It should now contribute to this PR's status checks. Update: The results are in! |
...baselines/reference/JSDocParsing/DocComments.parsesCorrectly.typedefTagWithChildrenTags.json
Show resolved
Hide resolved
@rbuckton Here they are:Comparison Report - master..35282
System
Hosts
Scenarios
|
@typescript-bot perf test |
Heya @rbuckton, I've started to run the perf test suite on this PR at ae7c638. You can monitor the build here. It should now contribute to this PR's status checks. Update: The results are in! |
@rbuckton Here they are:Comparison Report - master..35282
System
Hosts
Scenarios
|
What's the benefit of having to use |
The current factory methods mark nodes as For example, the parser now utilizes a custom TypeScript/src/compiler/parser.ts Lines 695 to 716 in 086633f
The existing factory methods will still be available for some period of time with deprecations: https://github.com/microsoft/TypeScript/blob/086633f4db521efb6bc25aadaca68f457e56a95b/src/compat/deprecations.ts. This model allows us to introduce deprecations with different levels of aggressiveness:
|
@typescript-bot perf test |
# Conflicts: # src/compiler/factoryPublic.ts # src/services/codefixes/convertToAsyncFunction.ts # src/services/codefixes/helpers.ts # src/services/codefixes/inferFromUsage.ts # src/services/signatureHelp.ts # tests/baselines/reference/api/tsserverlibrary.d.ts # tests/baselines/reference/api/typescript.d.ts
# Conflicts: # src/compiler/binder.ts # src/compiler/checker.ts # src/compiler/factory.ts # src/compiler/transformers/declarations.ts # src/compiler/transformers/es2015.ts # src/compiler/transformers/module/module.ts # src/compiler/transformers/module/system.ts # src/compiler/transformers/taggedTemplate.ts # src/compiler/transformers/ts.ts # src/compiler/utilities.ts # src/compiler/visitor.ts # src/harness/vfsUtil.ts # src/services/codefixes/addMissingAsync.ts # src/services/codefixes/convertToMappedObjectType.ts # src/services/codefixes/helpers.ts # src/services/completions.ts # src/services/refactors/generateGetAccessorAndSetAccessor.ts
I'm going to merge master into this one more time to make sure there aren't any new changes that need to be captured, then I can merge this to master. |
Nice work! Was a decision made on the renaming proposal? |
Yes, we merged #39058 which used the more consistent and clearer names for factory methods. |
Most ts.create* and ts.update* functions are deprecated as of TypeScript 4.0. The current plan is to emit noiser and noisier warnings with each version until the deprecated functions are removed in 4.3. See this pull request for more details: microsoft/TypeScript#35282 Some method signatures and names have changed slightly. The easy way to update is to use the global ts.factory object. TypeScript transformations also have access to a NodeFactory in their transformation contexts. Note that we still use the deprecate getMutableClone function because its replacement is not included in the TypeScript declarations. See microsoft/TypeScript#40507
Most ts.create* and ts.update* functions are deprecated as of TypeScript 4.0. The current plan is to emit noiser and noisier warnings with each version until the deprecated functions are removed in 4.3. See this pull request for more details: microsoft/TypeScript#35282 Some method signatures and names have changed slightly. The easy way to update is to use the global ts.factory object. TypeScript transformations also have access to a NodeFactory in their transformation contexts. Note that we still use the deprecate getMutableClone function because its replacement is not included in the TypeScript declarations. See microsoft/TypeScript#40507
Most `ts.create*` and `ts.update*` functions are deprecated as of TypeScript 4.0. See this pull request for more details: microsoft/TypeScript#35282 Additionally, some API calls (especially `createImportClause`) needed to be updated because apparently the API changed.
This overhauls our node factory API to provide the following benefits:
Node
is stable between the parser, checker, and transformers to reduce polymorphism, as well as to provide a "single source of truth" with respect to how nodes are created.binder.ts
andtransformer.ts
and ensures that transform flags will always be correctly aggregated during transformations.In the future, this will also allow us to better isolate which transformer was responsible for creating nodes, as well as to possibly leverage transformers outside of the emit pipeline.
As part of these changes, all of the existing
create*
andupdate*
factory functions in thets
namespace are being moved to aNodeFactory
object that can be created via the internalcreateNodeFactory
function. Authors of transformers can access theNodeFactory
associated with their transformation via theTransformationContext
provided to their transformer:In addition, the
ts.factory
object is a singletonNodeFactory
instance, which can be used to create synthetic nodes outside of a transformer.So as not to break compatibility with older API consumers, the existing
create*
andupdate*
factory functions will also be available to consumers of the TypeScript language service and the TS Server API. However, these functions are considered deprecated, and will be phased out using the following (tentative) plan:@deprecated
JSDoc comment and are no longer used by TypeScript internals.console.log
-like logger is available).Remaining TODO items:
createParen
to a more expressive (and less ambiguous) name such ascreateParenthesizedExpression
.readonly
, as one goal is to treat a TypeScript AST as immutable (and should be "changed" via visitors).