Skip to content
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

Latest API changes make usage more complicated #122

Closed
squidfunk opened this issue Aug 27, 2023 · 3 comments
Closed

Latest API changes make usage more complicated #122

squidfunk opened this issue Aug 27, 2023 · 3 comments

Comments

@squidfunk
Copy link
Sponsor

First of all, thanks for this super useful project! However, with the latest upgrade to version 3 and TypeScript 5, the signature seems to have changed. Before, one could just export an object referencing multiple transformers, something like:

export function factory() {
  return {
    after: [transformEnum, transformPath],
    afterDeclarations: [transformPath]
  }
}

I could then just use:

"plugins": [
  { "transform": "my-transformer" }
]

This seems not to be possible anymore. From v3 on, the after and afterDeclaration keys must be explicitly set in tsconfig.json, which makes multiple entries necessary. Additionally, transformer factories cannot return arrays of transformers, which means that I would now have to define 3 entries in tsconfig.json instead of one:

"plugins": [
  { "transform": "my-transformer/enum", "after": true },
  { "transform": "my-transformer/path", "after": true },
  { "transform": "my-transformer/path", "afterDeclarations": true }
]

I've read through the latest changes and release notes and can't manage to find the rationale behind this change. Was this forgotten or deliberately decided against?

@squidfunk
Copy link
Sponsor Author

I've found a workaround that lets me at least compose multiple transformers, but you still need to define it twice in tsconfig.json. It would be amazing if ts-patch could handle that for us, as it did in v2. Workaround:

import { chain } from "radash"
import { Node, TransformerFactory } from "typescript"

function compose<T extends Node>(
  ...transformers: TransformerFactory<T>[]
): TransformerFactory<T> {
  return context => chain(
    ...transformers.map(fn => fn(context))
  )
}

export function factory() {
  return {
    after: compose(transformEnum, transformPath),
    afterDeclarations: compose(transformPath)
  }
}

samchon added a commit to samchon/typia that referenced this issue Oct 22, 2023
As TypeScript v5.3 has break change on compiler API, current version of `ts-patch` does not work on the `typescript@5.3` update. Therefore, blocked the TypeScript v5.3 update by configuring `peedDependencies` of `package.json`.

This limitation would be resolved when `ts-patch` starts supporting the TypeScript v5.3 update.

nonara/ts-patch#122
samchon added a commit to samchon/nestia that referenced this issue Oct 22, 2023
As TypeScript v5.3 has break change on compiler API, current version of `ts-patch` does not work on the `typescript@5.3`.

Therefore, I've blocked the TypeScript v5.3 update by configuring `peedDependencies` of `package.json`.

This limitation would be resolved when `ts-patch` starts supporting the TypeScript v5.3 update.

  - Related issue: nonara/ts-patch#122
@nonara nonara closed this as completed in aabf389 Dec 5, 2023
@nonara
Copy link
Owner

nonara commented Dec 5, 2023

Hi @squidfunk! Thanks for the report.

The backstory here is that this was originally based on ttypescript. Chaining wasn't supported according to the original type:

https://github.com/cevek/ttypescript/blob/master/packages/ttypescript/src/PluginCreator.ts#L37C1-L41C2

Also, allowing multiple keys (ie. after and before) to run from the same plugin entry wasn't supposed to be supported either, according to official documentation.

What we did with the latest breaking version was rewrite a lot of things and make them conform to spec.

However, chaining support is definitely valuable, so I added that back in in the new v3.1.0 release. I wasn't aware that chaining was supported before, due to the types indicating that it should not be.

The decision to to make keys explicit was intentional, however. The truth is, the config design is poorly thought out and tedious. Using "before", "after", etc as their own keys is terrible, and putting these inside of the official plugins area was also a very bad decision, imo.

With that said, official documentation has always required that "before", "after", "afterDeclarations" be explicitly specified (empty defaults to "before"), and it also requires that users add unique entries for each. As a result, making the code line up with the documentation was a necessary step. But I agree that it's just poorly designed.

What will be coming in the future is a much easier and much better system, which allows bundling multiple transformers in different phases in single plugin modules. Those modules will be specified just once in config. They'll also be able to include language service plugins and more. For the time being, we simply took steps necessary to make sure all types and documentation were accurate.

You can go ahead and drop your chaining work around. Let me know if you have any issues!

@squidfunk
Copy link
Sponsor Author

You are awesome – thanks for putting in the work, making TypeScript more joyful to work with ❤️

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

No branches or pull requests

2 participants