jakebailey commented 1 year ago

In the TypeScript compiler, we have the concept of a "transformer". A transformer is a function which "transforms" one AST to another. TypeScript ships many such transforms, including all of the various transformation steps needed to downlevel newer syntax to older syntax, to convert from import/export to require/module.exports, and so on. Even the declaration emitter is a transformer, stripping away function bodies, filing in types, etc.

Since TypeScript 2.3, TypeScript has had the following API:

interface Program {
    emit(
        targetSourceFile?: SourceFile,
        writeFile?: WriteFileCallback,
        cancellationToken?: CancellationToken,
        emitOnlyDtsFiles?: boolean,
        customTransformers?: CustomTransformers, // <--- 👀
    ): EmitResult;
}

interface CustomTransformers {
    before?: (TransformerFactory<SourceFile> | CustomTransformerFactory)[];
    after?: (TransformerFactory<SourceFile> | CustomTransformerFactory)[];
    afterDeclarations?: (TransformerFactory<Bundle | SourceFile> | CustomTransformerFactory)[];
}

This API allows users to provide a set of "custom transformers" at emit time. These transformers run alongside TypeScript's own transformers, either before or after, depending on where they are placed in CustomTransformers.

However, while this API exists and does work, a major gotcha is that custom transformers can only be provided to the API. There is no way to use custom transformers if you want to use tsc alone.

This has been a long-standing issue; #14419 is the fourth most upvoted issue on the repo.

What do people use custom transformers for?

This list can go on and on, but transformers are commonly used these days for:

Generating serialization / deserialization / runtime type checks
- typia,ts-runtime-checks, which top the benchmark charts, but there are many more
Powering internationalization
- formatjs
Generating database code
- ts-graphql-plugin
Transforming paths (please don't do this one, though 😅)

I also know of many ideas which want to be implemented via plugins, but are not willing to do so without official support, including other runtime type checks, tracing code, and so on.

How do users currently use custom transformers?

Given the current constraints of custom transformers, there are two ways that downstream users have been able to use them:

Build TypeScript code via the API, either directly or by using some system which uses the API.
- webpack, rollup's TS plugins, nx, all provide configuration which allows users to specify a set of custom transformers.
Patch TypeScript.
- ttypescript, ts-patch wholly replace TypeScript in a user's workflow.

The first one is obviously fine. The latter, eek!

What changed?

In TypeScript 5.0, we shipped the biggest architectural / packaging change in TypeScript's history; conversion of the entire project to modules, including the bundling of our code. The relevant side-effects are twofold:

The structure of tsc.js (and other outputs) changed massively. This broke anyone who was patching the contents of the TypeScript package.
The API is no longer runtime patchable; we use esbuild to bundle our code and the objects it creates are not "configurable", correctly emulating how "real" ES modules would behave. Any runtime patches were already extremely fragile, only functioning due to the structure of our old namespace emit.

In response to this, I did an analysis of as many TS "patchers" as I could find, and found that there are actually very few reasons to patch TypeScript at all:

ttypescript/ts-patch, which seem to be almost exclusively used to enable the use of custom transformers.
Language service plugins, mainly injecting their own module resolution and custom source file types (vue/volar, css, etc).
Build systems like heft.
Yarn PnP.
Those who want to tree shake out our parser (prettier).

For 5.0, I was able to fix many of these projects (or at least guide / warn maintainers ahead of time), but many of these patchers have no viable alternative.

We have been seriously considering ways that we can approach the problems that remain in hopes that we can eliminate the need for people to patch TypeScript. This includes (no promises):

Custom transformers.
Custom module resolution.
A future API that actually allows a consumer to tree shake out just a parser (or similar).

This issue intends to address the first bullet.

A conservative, targeted proposal

In short, my proposal is to add to TypeScript the ability to define "custom transformer plugins". This new functionality is targeted and minimal. Its only intent is to satisfy those who just want to be able to add custom transformers.

Many details still have yet to be expanded on, but in short:

These plugins are placed into the plugins array of compilerOptions, with a discriminator like "type": "transformer".
- This is the approach taken by ttypescript/ts-patch, and offers up a future base for additional tsc plugins (e.g. file watching plugins, module resolution plugins).
- The choice in discriminator ("type": "transformer", for now) is intended to avoid conflicting with existing plugins defined by ttypescript or ts-patch.
Plugins would take a form similar to language service plugin, receiving the ts object, returning a factory that can be used to create a CustomTransformers object. The factory would receive the Program, which is required for any type-using plugins.
- The ts object must be a part of the API for similar reasons to the language service plugins; the APIs available within the core compiler are very, very limited, and the bundle is not the same as typescript.js.
- The plugin object entry is passed to the plugin for extra configuration, like LS plugins.
A new tsclibrary.d.ts file is added, describing the limited API within tsc. This is similar to tsserverlibrary.d.ts.
When invoking tsc, you must pass --allowPlugins to opt into executing plugins. When using the API or tsserver, plugins are enabled by default.
- No sandbox is present otherwise. If you currently use a patching-based method to use plugins, you're already taking matters into your own hand. If you currently use webpack or rollup, your config files are executable anyway.
- --allowPlugins is also a part of the watch plugin PR.
If multiple plugins are used, their returned CustomTransformers are merged.

There are almost assuredly other things that people want to be able to do with custom transformers or other plugins, however, I believe that we can serve the vast majority of those who want custom transformers with this API, and I do not want to let perfect be the enemy of the good.

In the future, we can expand plugins to do more, allowing the customization of other aspects of TypeScript, or simply add on to what custom transformer plugins can do. Prior research has shown that there are more interested parties, which I believe that we can eventually support.

An example

With the above proposal, a tsconfig.json would look like this:

{
    "compilerOptions": {
        "plugins": [
            { "type": "transformer", "path": "@jakebailey/cool-transformer" }
        ]
    }
}

The transformer would look something like:

import type * as ts from "typescript/tsclibrary";

const factory: CustomTransformersModuleFactory = ({ typescript }) => {
    return {
        create: ({ program }) => {
            return {
                before: [(context) => (file) => {/* ... */}],
            };
        },
    };
};

export = factory;

Of course, the details may change.

Unsolved problems

It's very unfortunate to have to define yet another random d.ts API used for one thing.
- Can we do better?
- What if the executables were ESM?
Like LS plugins, tsc plugins would need to be CJS.
- Can we instead make use of import?
- How do we do async? (This may already be "solved" in a prior WIP for plugins)
Should we do any sort of performance metric counting for plugins?
- Like LS plugins, can we just say "no warranty" when plugins are involved?
How much of a problem is it going to be to need to use the typescript namespace passed into the plugin?
- What happens to existing transformers which mistakenly import typescript.js's API directly, even though that may not have always worked?
- Will plugins unknowingly use the typescript.js API and rely on it, assuming users won't use tsc?
- Should we try and attempt converting our binaries/libraries to ESM first?
Is the proposed API too factory-y?
Should we even do this?

Please give feedback!

If you are currently using transformers, creating transformers, or otherwise, I would appreciate your feedback.

jakebailey commented 1 year ago

Preemptive @nonara ping; I suspect you have feedback! 😄

timocov commented 1 year ago

Hey @jakebailey,

Thank you for this proposal! I personally like the approach (it feels similar to ttypescript), but I have a question - it seems it is not possible to provide options to a "transformer" from the config, right? Would it be possible to allow to define something like options/params/config next to type and path fields that the compiler will pass as-is (without any transformation/validation) to the transformer function?

Quick intro where I came from

I'm the author of 2 optimization-related transformers: - https://github.com/timocov/ts-transformer-minify-privates - https://github.com/timocov/ts-transformer-properties-rename They both allow you to minify properties of objects (either private or internal) and it is achieved by adding a prefix to "all properties that should be minified" so you can use these prefixes in tools like `terser` or `uglifyjs` to minify i.e. mangle these properties. While the default prefixes (`_private_` and `_internal_`) are used in majority cases, there are few cases when people prefer to use different prefixes for whatever reason (e.g. to sync values in their config files). Thus it would be quite beneficial to have such option.

Thanks!

jakebailey commented 1 year ago

Would it be possible to allow to define something like options/params/config next to type and path fields that the compiler will pass as-is (without any transformation/validation) to the transformer function?

Oops, I should have explicitly defined that; yes, any other options are passed along to create, just like language service plugins.

export type CustomTransformersModuleFactory = (mod: { typescript: typeof ts }) => CustomTransformersModule;

export interface CustomTransformersModuleWithName {
    name: string;
    module: CustomTransformersModule;
}

export interface CustomTransformersModule {
    create(createInfo: CustomTransformersCreateInfo): CustomTransformers;
}

export interface CustomTransformersCreateInfo {
    program: Program;
    config: any;
}

patdx commented 1 year ago

Why wouldn’t it support esm? Is it because the initializing code of tsc is synchronous? It may be nice if it could support esm from the start.

jakebailey commented 1 year ago

Why wouldn’t it support esm? Is it because the initializing code of tsc is synchronous? It may be nice if it could support esm from the start.

Yes, correct, in its current form. The same currently applies to tsserver plugins.

We'd need dynamic import and that doesn't fit nicely into our current architecture. But, I need to go check what was done in the previous attempt as I believe it may have been solved there.

samchon commented 1 year ago

It seems great. TS 5.2 update is expected.

samchon commented 1 year ago

This is just my opinion - @jakebailey

Support both `TransformerFactory` and `CustomTransformerFactory`

Looking at changed codes, I think it may possible to support both TransformerFactory and CustomTransformerFactory. There're some useful libraries using ttypescript, but some of them had stopped maintaining. I think supporting both interfaces maybe helpful for them.

`--allowPlugins` to be default `true` or just remove it

To use transformation based libraries like my typia (or nestia), users must configure tsconfig.json file with plugin property. Configuring the plugin property by themselves mean that they have already agreed to using plugin transformation. In my opinion, typing --allowPlugins CLI is just annoying thing for users.

I hope it to be default true, or just remove it.

jakebailey commented 1 year ago

Looking at changed codes, I think it may possible to support both TransformerFactory and CustomTransformerFactory. There're some useful libraries using ttypescript, but some of them had stopped maintaining. I think supporting both interfaces maybe helpful for them.

Those transforms are unlikely to work because they import typescript directly. The APIs available within tsc are very limited, no methods on Node, Type, etc, and it's very possible for a user to accidentally get into a setup where a plugin may import a different version of typescript than is loaded. These plugins need to be loaded in the language service as well, which for most users is guaranteed to not be TypeScript within the workspace, but instead be the one included in VS Code and therefore cause two different versions of TypeScript to be active in memory and potentially have mismatching behavior.

To use transformation based libraries like my typia (or nestia), users must configure tsconfig.json file with plugin property. Configuring the plugin property by themselves mean that they have already agreed to using plugin transformation. In my opinion, typing --allowPlugins CLI is just annoying thing for users.

This comes down to the security tradeoffs. In the API, you're already running code, so the flag isn't needed. In the language service, we already load plugins automatically and in VS Code require that you "trust" the workspace before starting tsserver. There is no such equivalent for tsc and without --allowPlugins, this would be the first instance of tsc arbitrarily executing code.

I should also note that it's possible that this proposal is not implemented at all. There are still strong arguments against doing this sort of thing whatsoever. The use cases may not outweigh the downsides; it may also be the case that this feature would be avoided by most because this means they cannot use any transpilers except TypeScript itself.

canonic-epicure commented 1 year ago

--allowPlugins to be default true or just remove it

To use transformation based libraries like my typia (or nestia), users must configure tsconfig.json file with plugin property. Configuring the plugin property by themselves mean that they have already agreed to using plugin transformation. In my opinion, typing --allowPlugins CLI is just annoying thing for users.

I hope it to be default true, or just remove it.

+1 for this, --allowPlugins is clearly an overkill. Its not a nuclear rocket launch, where you have to use 2 different keys from 2 persons, rotate them simultaneously etc. If user has specified the transformer in the "tsconfig.json" I believe his/her intention is clear enough.

sosoba commented 1 year ago

ttypescript passed another useful argument to the plugin. Method addDiagnostic which allows plugins to report errors and warnings to a place in the code in a standard way:

    helpers?: { ts: typeof ts; addDiagnostic: (diag: ts.Diagnostic) => void }

It seems to me that this feature should be in CustomTransformersCreateInfo too.

amanteaux commented 1 year ago

Another use case for custom transformer is to provide dependency injection at compile time. This is what is partially done in https://github.com/wessberg/DI-compiler

43081j commented 1 year ago

I've actually pushed for this many times over the years as I've written a fair few transforms, most of which are for codegen stuff (e.g. replacing decorators with equivalent code).

However, I think i'd vote against it these days.

If we open the flood gates here, we're opening ourselves up to inconsistent behaviour and syntax across any two typescript projects - which seems high risk to me.

The usual culprits that already implement a lot of "magic" (frameworks, codegen projects, gql, etc) i'm sure would make use of it and build their own "flavour" of typescript soon enough. Which would lead to fragmentation of an otherwise pretty concrete ecosystem/community.

On top of that, typescript was always meant to be "javascript with types" (i.e. just some annotations we can strip away). That kind of goes away if we open up to custom syntax.

I'd love to be able to simplify some of my builds by this being possible, but i would rather put up with needing my own build script to avoid the above.

anyone who really wants to transform the AST can still throw a small build script together. but that slightly higher bar is why we still have a consistent syntax overall.

samchon commented 1 year ago

Summary

TS 5.2 transform API removed type info.

I'd tried to adjust this TS 5.2 transform API in my llibraries typia and nestia.

However, it was not possible because most methods and properties are erased from ts.Node and ts.Type in TS 5.2 transform API. By such removes, even checking number type became impossible, due to ts.Type.getFlags() method has been erased. Another words, I can't support runtime validation feature through TS 5.2 transform API.

I asked the reason why in the related PR, but got answer that it's for performance enhancement (https://github.com/microsoft/TypeScript/pull/54278#issuecomment-1550830073). Unfortunately, it just sounds me like "I removed the seatbelt and fuel efficiency went up." Considering the purpose of the TypeScript project, I think this is nonsense.

@jakebailey I'm sorry for criticizing even from the 1st PR. However, type info is very important feature for me. I just hope TS 5.2 transform API to provide full TS API specs.

jakebailey commented 1 year ago

I don't understand the above; clearly all type information is available within tsc, it's just not as convenient as how it is within the wider services API. If we were to do this, we would of course want to ensure that things are accessible; that is the point after all. I mentioned some of this preciously.

I'll also say that I don't understand how what you're saying is accurate when the patching libs still operate within the same bounds as tsc already has. How are they getting the methods, when such methods don't even exist within tsc?

samchon commented 1 year ago

@jakebailey This is a list how TS 5.2 transform API breaks transformer libraries:

As ts.Node.getSourceFile() and ts.SourceFile.getLineAndCharacterOfPosition() be erased, it became impossible to show exact (3rd party library defined) compile error position
As ts.Node.getSourceFile() be erased, it became impossible to support special transformation function in 3rd party library.
As ts.Node.getSourceFile() be erased, it became impossible to analyze import relationship in 3rd party library.
As ts.Type.isTypeParameter() be eerased, it became impossible to find or filter out generic argument.
As ts.Type.isLiteral() and ts.LiteralType.value be erased, unable to analyze literal type.
As ts.Type.isClass() be erased, unable to check class type.
As ts.Declaration.getChildren() and ts.Type.get(Apparent)Properties() be erased, unable to list up properties.
As ts.JsDocTagInfo, ts.Symbol.getDocumentationComment(), and ts.Symbol.getCommentTags() be erased, unable to analyze comments and comment tags.
As ts.Type.isUnionType() and ts.Type.isIntersection() be erased, unable to analyze union type.
As ts.Type.getFlags() be erased, unable to analyze exact value type like number.
As ts.Type.getSymbol() be erased, unable to get exact ts.Symbol value.
As ts.Type.isTupleType() be erased, unable to analyze tuple type.
As ts.Type.isArrayType(), ts.Type.isArrayLikeType() and ts.Type.getNumberIndexType() be erased, unable to analyze array type.
As ts.Node.getFullText() be erased, unable to get full name.

Also, you're saying that all type informations are available in typescript/lib/tsclibrary, and additional properties and methods in typescript is just for convenience. However, note that, 3rd party transformer libraries had used only typescript for many years, not typescript/lib/tsclibrary. It is not just a problem of convenience, but 3rd party transformer library developers don't know typescript/lib/tsclibrary at all.

Furthermore, as you're a member of TypeScript compiler developer, you can feel that everything is possible in typescript/lib/tsclibrary, and typescript is just for convenience. However, you know what? TypeScript had not provided any detailed documents about compiler API. Therefore, third-party developers just looked at the names of objects, methods, and properties of APIs that existed in typescript, and learned how to use them by randomly inferring and experimenting, especially depending on TypeScript AST viewer. Even typescript was terribly difficult for me, because everything was black-box.

In such circumstance, you're just going to adjust breaking change that 3rd party libraries's developers never had experienced. I don't know how much performance benefit would be in the typescript/lib/tsclibrary, but I hope you to respect legacy 3rd party libraries had utilized ttypescript and ts-patch. We had utilized typescript instead of typescript/lib/tsclibrary too much time.

Summary
- TypeScript had not supported any detailed documents about compiler API
- For many years, 3rd party libraries have utilized not typescript/lib/tsclibrary but typescript
- Therefore, TS 5.2 transform API would become break change, even without any detailed documents
  - You're saying that everything is possible in typescript/lib/tsclibrary
  - And typescript is just for convenience
  - However, even typescript was terribly difficult for me, due to blackbox development
- Hope to consider such 3rd party circumstances, and please just provide typescript transform API

p.s.) Just providing more convenient methods and properties, it really affects to the performance? I can't understand it.

samchon commented 1 year ago

@jakebailey As you've said typescript is just convenient and everything can be done typescript/lib/tsclibrary, I've studied TypeScript codes for a day. During that times, I could find alternative way like below. By the way, the alternative way means to define duplicated code with TypeScript core codes that are not exported.

ts.Node.getSourceFile() and ts.Node.getFullText() can be manually implemented, but it would better to TypeScript exports src/compiler/utilities.ts without @internal tag like getSourceFileOfNode() function.
ts.Type.isTypeParameter() can be replaced by & operation (ts.TypeFlags.TypeParameter)
ts.Type.isLiteral() also can be replaced by & operation
ts.Type.isIndexType(), ts.Type.isIntersection() types are also possible
ts.Type.isArrayType() and ts.Type.isTupleType() are possible by ts.TypeChecker

Besides, there are some features that could not implement easily, or impossible. Traveling TypeScript source codes, I can sure that there're much more features that could not be implemented by 3rd party library. I hope you to consider such aspects, and want at least one of them: just allow typescript definitions, or export internal compiler and service functions.

Also, I'm sorry for say like that:

Everything by typescript/lib/tsclibrary, it was possible because you're a core developer of TypeScript.

Too many features are @internal tagged and even not exported.

ts.SourceFile.getLineAndCharacterOfPosition() has deep dependency on internal functions. Especially about lineMap property, it does not exist on the ts.SourceFile type due to @internal tag. Also, it may possible to copying and pasting same code on my transformer library, but cannot sure it is right behavior or not.
ts.Type.isClass() can't be implemented because key types are @internal tagged.
ts.Symbol.getDocumentationComment() and ts.Symbol.getJsDocTags() also related with deep internal functions.
ts.Node.getChildren() has the most complicate relations with internal functions
About ts.transpile() function, I just gave up

jakebailey commented 1 year ago

re: https://github.com/microsoft/TypeScript/issues/54276#issuecomment-1551366861

Use ts.getSourceFileOfNode and ts.getLineAndCharacterOfPosition.
Use ts.getSourceFileOfNode.
Use ts.getSourceFileOfNode.
Check type.flags & TypeFlags.TypeParameter.
Check type.flags & (TypeFlags.StringLiteral | TypeFlags.NumberLiteral | TypeFlags.BigIntLiteral). LiteralValue's value property is public.
This can be checked via objectFlags. We could export a helper for this.
I'm not convinced getChildren should be used in a transform at all. Our transforms do not use this helper. You probably want to be using explicit visit functions. For the properties, call the checker, which has getPropertiesOfType and getAugmentedPropertiesOfType.
I don't think a transform should need getDocumentationComment. That's for the language service for displaying in tooltips and such. There are other ways to obtain JSDoc info, too.
Check type.flags & TypeFlags.Union or type.flags & TypeFlags.Intersection.
Just access type.flags.
Just access type.symbol.
Call typeChecker.isTupleType.
Call typeChecker.isArrayType, typeChecker.isArrayLikeType, or typeChecker.getIndexTypeOfType.
Call ts.getSourceFileOfNode and slice the text by node.pos and node.end.

When you operate within the core compiler, you have to play by the core compiler's rules. These are all things that we do on a regular basis. If there is an API missing, then we can add it.

But pushing FUD on multiple issue trackers is really not helping, in fact, it's mostly convincing me that I shouldn't have made this proposal at all.

samchon commented 1 year ago

Okay, I will argue only in here issue. Sorry for multiple articles' comments

samchon commented 1 year ago

@jakebailey How to replace ts.Symbol.getJsDocTags() method?

Also, In my case, ts.symbol.getDocumentationComment() is required for JSON schema definition.

jakebailey commented 1 year ago

I suspect via #53627 or the .jsdoc property on the declarations.

nonara commented 1 year ago

Looks interesting, @jakebailey! Thanks for looping me in. I'll share a few thoughts below.

wholly replace TypeScript in a user's workflow ... eek!

I understand where you're coming from. I think in a broad sense, I probably wouldn't classify what's being done as solely this.

What we're essentially doing is injecting some additional code into the underlying file to extend its capability. In the case of tsc it's been a little more extreme, in that we have to splice together logic from two modules.

This can also happen in memory, where we're not actually replacing anything, which has been the practice for ttsc and is now integrated into tsp as well.

Moving forward, our methodology is further simplified as simply running transformers on the compiler to add functionality.

With that in mind, I don't think I'd necessarily agree that this is "eek" (😂), as I think that has some very real value in specific cases. If memory serves, IDEs like JetBrains (at least at one point) were doing similar.

That said, when confronting the question of whether it should be necessary to actually patch ts for something that has been so widely requested and is now widely used, I definitely agree that it makes sense to take a closer look. On the surface, it seems a bit extreme to need to patch ts to do it.

Should We?

"Your scientists were so preoccupied with whether or not they could, they didn’t stop to think if they should" — from the great philosophical opus, "Jurassic Park"

Certainly, if it was simple to fully support, it would be an easy answer, but in this case, you face the trade-off of either sacrificing the reduced size of tsc (presumably also sacrificing some performance), or in offering something which has limitations that prevent many use-cases.

One possible option I might consider would be creating a separate, full binary (eg. tscp.js), which has full API support for plugins. Obviously, though, this all depends on what your priorities are, and I'm not advocating for that. On a simply surface level, this would ameliorate issues and provide a full solution.

A secondary option is to simply do it how you've proposed, and tsp can extend capability for those who need it.

Issues with New TS 5

One quick side tangent, as I think it has been widely misunderstood (understandably) across the community.

The changes made to TS 5.0 were actually not that much of a challenge to accommodate. In fact, esbuild made it a bit easier to identify the surgical points.

Ultimately, there were a couple of main reasons it took me a a while to do. Mainly, I took it as an opportunity to do some major rewrites I had planned for probably two years. This was in part because it was required.

That aside, I've also just been slammed. I did, however, get a working beta out ASAP.

I can understand why people got nervous, but putting it into perspective, the worst that happened was people had to wait a little bit to try the latest and greatest. This is obviously not ideal, and part of the reason for the rewrite is to make it so that doesn't happen moving forward.

Anyway, I don't want to hijack the thread on that.

Further Responses

A future API that actually allows a consumer to tree shake out just a parser (or similar).

Sounds very interesting! Looking forward to seeing what you come up with.

with a discriminator like "type": "transformer"

This sounds great. I didn't choose the current config format, and I don't like how it's laid out.

I was planning on moving plugin config for future tsp outside of tsconfig entirely, but I'd be glad to conform to whatever is officially released.

A new tsclibrary.d.ts file is added, describing the limited API within tsc. This is similar to tsserverlibrary.d.ts.

I like this approach.

How much of a problem is it going to be to need to use the typescript namespace passed into the plugin?

This one is interesting. One thing that's very important with writing proper transformers is to use the passed ts instance. Very few transformers actually do this, and it can cause issues in numerous cases.

I think the most important part would be good documentation.

One possibility (though I don't know if it's worth it or a good idea) would be to "sign" root elements, like SourceFile and possibly Program, so you could throw if they tried to pass to an incompatible instance.

ESM

This part is certainly tricky. I ended up getting esm transformer support to work in tsp (still have to address a memory leak issue) by transpiling to CJS on-the-fly.

Summing it up

It's an interesting proposal, and I love the idea of a better config format. Your research and thought process so far is definitely on track!

Looking forward to keeping up with it.

As for the noise, just a word of encouragement — keep your head up! I know I don't need to tell you, but it's a complex subject with a lot of specialized knowledge, which broadens the field of those who'll misunderstand. I certainly know how frustrating the feedback can be when people don't fully grasp the issues, especially when that's matched by boldness and curt expression.

That said, I can't begin to imagine what you all have to deal with on a daily basis. I've seen some threads!

I think I can speak for a good portion of the silent majority who appreciate the efforts to investigate this further and to come up with a fully well-thought out determination on approach, and I appreciate the invitation to weigh in.

jakebailey commented 1 year ago

With that in mind, I don't think I'd necessarily agree that this is "eek" (😂), as I think that has some very real value in specific cases. If memory serves, IDEs like JetBrains (at least at one point) were doing similar.

Truthfully, my main motivation here is that I would really like to eliminate all of the external need to patch the TypeScript package; the fact that people do this limits what we can do in our package, e.g. we can't think about doing any sort of minification because doing so means that either it'll break .patch files (which work on lines; e.g. yarn's patching), potentially mangle variable names (which a syntactic transform may break with), and remove comments (e.g. the hack that prettier uses to extract just the parser).

Certainly, if it was simple to fully support, it would be an easy answer, but in this case, you face the trade-off of either sacrificing the reduced size of tsc (presumably also sacrificing some performance), or in offering something which has limitations that prevent many use-cases.

Size wise, I'm not worried. It's only 200KB or so in tsc, just the effect of not tree shaking due to any API potentially being used by a plugin. See the size report on my WIP PR: https://github.com/microsoft/TypeScript/actions/runs/4997656840#summary-13534656632

One possible option I might consider would be creating a separate, full binary (eg. tscp.js), which has full API support for plugins. Obviously, though, this all depends on what your priorities are, and I'm not advocating for that. On a simply surface level, this would ameliorate issues and provide a full solution.

Another option to "solve" this may be to try and restructure our API to make it easier to build custom CLIs that enable more features like plugins. Right now, I think that is challenging, but I think it could be very possible. I already want to try and ESM-ify the package.

One quick side tangent, as I think it has been widely misunderstood (understandably) across the community.

The changes made to TS 5.0 were actually not that much of a challenge to accommodate. In fact, esbuild made it a bit easier to identify the surgical points.

Yes, totally. Though, I can't say I'm super happy overall that people are using the // src/compiler/... comments for patching...

nonara commented 1 year ago

@43081j

If we open the flood gates here, we're opening ourselves up to inconsistent behaviour and syntax across any two typescript projects - which seems high risk to me.

I respect where you're coming from here, and I agree that this would not be a good outcome.

In this case, that should not be a risk, and I'll try to quickly explain why that is.

The Future of TSP

At this point, ts-patch has implemented the same in-memory feature of ttypescript and it has continued support, where tts is not being updated anymore. Becoming the central library of support allows us to reduce some ambiguity and positions us to finally create a better approach (especially with regard to things like the current mess of a config format with all the different "entry-points").

Note: We will still remain backward compatible to support old transformers

What I can tell you is that I plan to continue tsp regardless of what happens here, for a few reasons:

We want to continue to support legacy transformers
Current proposal looks like it will have limited API, making it still necessary for some use-cases
Our plan is to expand to something broader, which will allow for more powerful plugin packages

The new tsp plan should make it easy to package multiple transformers, easily filter diagnostics across language service & compiler, and even extend the compiler itself.

How It Fits with This Proposal

Our philosophy has always been to follow the lead of typescript. This is similar to how they defer, in some senses, upstream to TC39.

If the TypeScript team comes up with an official means of integrating transformers via config, we will gladly accommodate and encourage that format for transformers to be used moving forward.

However, we also maintain the importance of backward compatibility, so again, we'll make sure old-form transformers can still run.

So at the end of the day, I don't see that we have a divide, here.

If TS adds any form of support, that will be great, as it will require less from our side.

If what they add is sufficient for users, they can use that; otherwise, they can simply add tsp to the chain if you want or need to have something that goes beyond what ts offers.

samchon commented 1 year ago

import ts from "typescript";
import type lib from "typescript/lib/tsclibrary";

@jakebailey As typescript/lib/tsclibrary exists only as a type (d.ts file only), I've to import both typescript and typescript/lib/tsclibrary. By the way, as type definitions between typescript and typescript/lib/tsclibrary are different, I can't compile my project typia.

Only one step is left for TS 5.2. Am I missing something?

jakebailey commented 1 year ago

@samchon In order to correctly support the plugin model in this proposal, you need to consistently use the correct API across your project, passing the ts module provided by the create call around or just setting it to some global. This is a restriction already imposed on language service plugins; you just cannot mix and match the various bundles as they are all different, contain different things, have different classes which will fail instanceof, etc. You always need to be using the API object passed in by the plugin system, not any import.

samchon commented 1 year ago

@samchon In order to correctly support the plugin model in this proposal, you need to consistently use the correct API across your project, passing the ts module provided by the create call around or just setting it to some global. This is a restriction already imposed on language service plugins; you just cannot mix and match the various bundles as they are all different, contain different things, have different classes which will fail instanceof, etc. You always need to be using the API object passed in by the plugin system, not any import.

In my case, typia is used by both transformation, generation and library mode. As you know, transformation mode means using transform API. Generation mode means generating duplicated TypeScript files with transformation. Library mode means used by other library by importing.

Transformation mode: https://typia.io/docs/setup/#transformation
Generation mode: https://typia.io/docs/setup/#generation
Library mode for SDK generation: https://nestia.io/docs/sdk/sdk/

By the way, types of typescript can't be assigned to typescript/lib/tsclibrary. In that case, must I make duplicated codes to support TS 5.2 transform API? Or is there any other way?

jakebailey commented 1 year ago

That I'd need to think about; that's part of my "it's unfortunate to have yet another declaration" problem. But theoretically, you could scope your project down to only use what's available in tsclibrary and typescript should be a superset that is assignable down to tsclibrary.

I do want to get feedback on this proposal from others, so I don't want to clog this thread up with specifics of your project, however.

nonara commented 1 year ago

@jakebailey

All great points. I take a pretty hard stance on this, which is that regarding anything not explicitly deemed public API, I have no reasonable expectation of this not changing at any point.

It's great that you guys consider broader tooling, and I think that's important, especially in situations like yarn or IDEs.

What we're doing, in my mind, has always been a somewhat experimental status way to do things you couldn't otherwise do, which is really what my vision for the future is. For example, it would provide an excellent way to make portable patches to test new features to the language in a proposal.

This does, of course, carry with it the expectation that it's up to the patch authors to keep up with new versions. We all have to do this with TS changes anyway, but there will definitely be more risk on this side of things. That should be understood, and I think it makes sense for me to add a better disclaimer to that effect in our documentation.

With that said, I do get that this can add some additional stress or perception of constraint on your side, which I don't want to be the case.

we can't think about doing any sort of minification because doing so means that either it'll break .patch files

This does underscore my thinking on it making sense to have a better approach to patching TS for those of us who need to do it. Ideally, it would be with a single tool, which is the hope.

The method I have in place now slices the file into its original pieces, and it essentially has them as memoized SourceFiles, which you can transform. This performs very well, and it makes it easy to modify things using transformers. This approach is also much more sturdy and requires less maintenance than .patch files.

Ultimately, the rewritten tsp will allow modules to easily take this info and transform exactly what you need. It also leverages caching well and allows in-memory changes, which should work well for IDEs, yarn, etc. I expect this should not only require less maintenance, but it should also be faster for them (after the first run). The trick is just ensuring it's planned out very well.

I can't say I'm super happy overall that people are using the // src/compiler/... comments for patching...

To clarify, I didn't expect that these would remain present. Prior to this, our boundary detection was more flimsy, using a regexp pattern, so it made sense to use it, but I had the expectation of possibly changing it later.

But your point on not being able to do things like minification is a very good one.

Two immediate options I can see would be to either publish a second package with the uncompressed versions or to include the uncompressed binaries in a subdir (if the added filesize was acceptable)

Potential Proposal

I think ultimately, the hope of entirely eliminating the need to modify typescript is not going to be realizable.

The cases in which it needs to be done are going to be rare and often crazy use-cases, but those often happen to pop up in places like yarn or major IDEs, which unfortunately mean that rarity doesn't make them rare in the wild. No doubt, more will emerge in time, which would likely push beyond the bounds of whatever generator may be built.

However, I also don't like the fact that this is a pain-point which hinders further progress on the development of the compiler.

I wonder if instead it makes sense to work out a standard, so we can have a more formalized pathway, that gives us a few guarantees we can hold to and alleviates the issues for your side.

I do think having a tool which allows augmenting TS via transformation-based patches is a great approach, with advantages over binary patches, including lower maintenance to keep up. The major hurdle there was performance, but our current approach handles that very nicely.

As far as a standard, the idea would be to do something which provides a few basic guarantees and – more importantly – that takes off the constraints from your side.

Here is a general example:

Add a second directory called lib-full, with just the js library binaries that remained un-minified.
These contain a comment in a special format which delineates between files (not tied to esbuild)
Each release could have a short description of notable changes to the internals, like files renamed, functions moved, etc.

I think a few things like this would make it easier for us and patch developers to keep up, but I welcome feedback on that or feasibility, especially of the third.

Again, if we were to consider this route, I'd stress to everyone (to posterity if this becomes a broader discussion) that the goal here is to make things easier for the TS team first and foremost, removing constraints on them from improving the compiler.

Second, standardizing a format for a tool that can be used across the board for augmenting the compiler will be beneficial to the user community.

treybrisbane commented 1 year ago

@marcj heads up - this may have implications for Deepkit. 🙂

uasan commented 1 year ago

We have been seriously considering ways that we can approach the problems that remain in hopes that we can eliminate the need for people to patch TypeScript. This includes (no promises):

It is likely that as part of these efforts, an API will be created that will allow not to override the methods of the FS in order to provide d.ts content for files with a custom extension? https://github.com/microsoft/TypeScript/issues/52983

jakebailey commented 1 year ago

@uasan No, I don't think so. That sounds very unrelated to me.

canonic-epicure commented 1 year ago

I'd like to address the concern about fragmentation of TS ecosystem after implementing this proposal.

Lets look at Haskell. They have "language extensions" , which can add new features to the language, up to modifying the grammar and adding new syntax. Historically, there is a Haskell 2010 standard and now they are discussing the Haskell 2020 standard which is basically a Haskell 2010 + a set of commonly used language extensions, which have proven to be useful.

All those experiments with the language were happening in the user-space, the core team didn't have to spent time on those. And then useful ones are becoming a new standard. Users have to opt-in for every extension, but all extensions are interoperable, because they all compiles down to the intermediate language.

This is an example of how a language ecosystem can evolve, requiring minimal effort from the core team and utilizing the full creativity power of the community. You can't move forward by standing on the same place.

TS transformers are much simpler than that, they are supposed to only modify the JS output. As in the Haskell case, there is an intermediate representation which is shared by all code (JavaScript). Users have to opt-in for every transformer, there's no risk of breaking any existing code.

I believe restricting the language diversity is something that limits the TypeScript usage in the long run.

dvins commented 1 year ago

All those experiments with the language were happening in the user-space, the core team didn't have to spent time on those. And then useful ones are becoming a new standard. Users have to opt-in for every extension, but all extensions are interoperable, because they all compiles down to the intermediate language.

This is an example of how a language ecosystem can evolve, requiring minimal effort from the core team and utilizing the full creativity power of the community. You can't move forward by standing on the same place.

Well said @canonic-epicure!

mindplay-dk commented 1 year ago

@jakebailey please don't be discouraged 🙂

Your efforts to push for something, anything to happen in this area is greatly appreciated! 🙏

I wouldn't personally expect TS to directly adopt any transformer format previously defined by the community - I understand this might create more work for the community already invested in certain transformer formats, and I do think there's some risk of fragmentation if existing transformers continue to use community formats not officially supported by TS. But I think that's the cost of making progress - and as @nonara said, priority is to avoid creating problematic constraints for the compiler team.

Thank you @nonara and @canonic-epicure for your measured comments 👍

mrkev commented 1 year ago

Should some solution for versioning be included at the plugin level, or would peerDependencies in package.json suffice?

I'm wondering if tsc would want to know what version of typescript its plugins expect. If the only utility for this is warning about unsupported versions, perhaps it's redundant though.

mindplay-dk commented 1 year ago

@mrkev TS is semantically versioned - so I'd assume any change to a public API is considered a breaking change.

The general problem with plug-ins is well known - and it's not really tsc's job to manage dependencies.

jakebailey commented 1 year ago

TS is semantically versioned - so I'd assume any change to a public API is considered a breaking change.

TypeScript is not semantically versioned; we just bump from 5.1 to 5.2, and when we hit 5.9 the next version will be 6.0. For the public API, we try and not break things, including using a special deprecation system to provide an early warning. But that system is not included in tsc (historically).

sosoba commented 1 year ago

IMHO plugin should not have a dependency on typescript. The actual reference is passed in the arguments: https://github.com/cevek/ttypescript/blob/835c77bfe1a84bdafc7e223ece93f8a5a949243c/packages/ttypescript/src/PluginCreator.ts#L47-L51

mrmckeb commented 1 year ago

Sorry I didn't leave feedback earlier - but this looks great. As far as I understand, this would allow plugins like (typescript-plugin-css-modules to emit type information outside of the API (and IDE)?

If my understanding is correct, I think this will lead to a landslide of interesting plugins.

I'm more than happy to put some time into prototyping around this too. I'm in Australia, but can always make time for US-friendly chats too.

My thoughts on the proposal:

I'd like to see --allowPlugins available as a tsconfig.json option.
Async support is more important than ESM from my experiences, but ESM support would be great, especially because a lot of projects are moving to ESM-only.
Native TypeScript support would be cool, as everyone likes no-build projects.
I think it would be great to capture plugin performance metrics, but that could be something that shipped later.

Should we even do this? Definitely ;)

jakebailey commented 1 year ago

Async support is more important than ESM from my experiences, but ESM support would be great, especially because a lot of projects are moving to ESM-only.

Can you clarify what you mean by "async"? The only place where async and ESM apply is in async loading of plugins (which is feasible though challenging, compared to making anything else async which is extremely far off).

sosoba commented 1 year ago

There is essentail os my personal compiler where I load plugins asynchronously:

// full or incremantal compiler:

const program = ts.createProgram(...);
const customTransformers = await loadPlugins(program, { ts, addDiagnostic });
const { diagnostics, emitSkipped } = program.emit(
  undefined, // targetSourceFile?: SourceFile,
  undefined, // writeFile?: WriteFileCallback,
  undefined, // cancellationToken?: CancellationToken,
  undefined, // emitOnlyDtsFiles?: boolean,
  customTransformers,
);

// watch compiler
const compilerHost = ts.createWatchCompilerHost(...);

const afterProgramCreate = compilerHost.afterProgramCreate;

compilerHost.afterProgramCreate = async (builderProgram) => {
  const customTransformers = await loadPlugins( builderProgram.getProgram(), { ts, addDiagnostic });

  const builderProgramEmit = builderProgram.emit.bind(builderProgram);
  builderProgram.emit = (targetSourceFile, writeFile, cancellationToken, emitOnlyDtsFiles, /*emptyCustomTransformers*/) => {
    const emitResult = builderProgramEmit(targetSourceFile, writeFile, cancellationToken, emitOnlyDtsFiles, customTransformers);
    emitResult.diagnostics = [
      ...emitResult.diagnostics,
      ...pluginDiagnostics,
    ];
    return emitResult;
  };

  afterProgramCreate(builderProgram);
};

mrmckeb commented 1 year ago

Can you clarify what you mean by "async"? The only place where async and ESM apply is in async loading of plugins (which is feasible though challenging, compared to making anything else async which is extremely far off).

Yes, sorry. The plugin I maintain (and you've helpfully contributed to) makes use of a number of dependencies that have been moving to async-only APIs. As the methods we're proxying aren't async, we can't make use of those APIs. Mainly getScriptSnapshot I think.

jakebailey commented 1 year ago

TypeScript internally is hugely massively far off from incorporating anything but the most basic async stuff, e.g. at most, I think we could do enough async to do a dynamic import of ESM (which we kinda do for language service plugins). The rest... I'm pretty sure is going to have to be synchronous. But, I haven't had time to go reread previous attempts at plugins to know the extent of async there.

clearfram3 commented 1 year ago

The 5.2 iteration plan lists this as an investigation. Does that mean this is only research or that some draft of the transformer api will be released with 5.2?

jakebailey commented 1 year ago

We're still thinking about this (and other plugins, long term); it's not guaranteed to be in 5.2 or even any release. There are great arguments both ways.

See also the most recent design meeting notes in #54502.

samchon commented 1 year ago

@jakebailey

In NestJS case, it guides user to compile TypeScript source codes through nest build command instead of tsc. I'm testing in NestJS, but it is hard to advance because of that. Can you give another option instead of --allowPlugins argument? At least allowPlugins configuration can be done in tsconfig.json file, it would much better I think.

jakebailey commented 1 year ago

I'm not sure what you mean; if someone is wrapping tsc, then they can pass the flag or use the API which doesn't have the flag.

The flag is there so that tsc is not arbitrarily executing code without permission. I personally would not be thrilled to check out an untrusted TS issue repro and be unable to compile it without potentially harming my system due to a malicious plugin.

samchon commented 1 year ago

Well, unfortunately nest build is not delivering arguments to tsc.

If no plan to support alternative option and it is clear policy, I'll send a PR to NestJS with explanation.

Thanks for replying.

jakebailey commented 1 year ago

I'll reiterate that this issue is only a proposal. We have not committed to this API, or even the feature at all.

I would not recommend working on making your projects compatible with this proposal or to send PRs to other projects until we actually know it's going to be in a release, unless you're only looking to provide feedback on said API or functionality.

lorenzodallavecchia commented 1 year ago

The flag is there so that tsc is not arbitrarily executing code without permission.

I'm not convinced about the "untrusted code" argument. Yes, the concern is real, but many build tools support plugins, like webpack, eslint, karma... even npm itself may install any declared package and use it in the scripts field of package.json, even automatically at install time. How is tsc different?

microsoft / TypeScript

A minimal custom transformer plugin proposal #54276

What do people use custom transformers for?

How do users currently use custom transformers?

What changed?

A conservative, targeted proposal

An example

Unsolved problems

Please give feedback!

Support both `TransformerFactory` and `CustomTransformerFactory`

`--allowPlugins` to be default `true` or just remove it

`--allowPlugins` to be default `true` or just remove it

Summary

microsoft / TypeScript

A minimal custom transformer plugin proposal #54276

What do people use custom transformers for?

How do users currently use custom transformers?

What changed?

A conservative, targeted proposal

An example

Unsolved problems

Please give feedback!

Support both TransformerFactory and CustomTransformerFactory

--allowPlugins to be default true or just remove it

--allowPlugins to be default true or just remove it

Summary

Support both `TransformerFactory` and `CustomTransformerFactory`

`--allowPlugins` to be default `true` or just remove it

`--allowPlugins` to be default `true` or just remove it