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

Enable defaultJSDocParsingMode of TypeScript v5.3 #134

Closed
samchon opened this issue Nov 8, 2023 · 10 comments
Closed

Enable defaultJSDocParsingMode of TypeScript v5.3 #134

samchon opened this issue Nov 8, 2023 · 10 comments

Comments

@samchon
Copy link
Contributor

samchon commented Nov 8, 2023

Outline

microsoft/TypeScript#55739

After TypeScript v5.3 being released, it will no more parse JSDocComments as default.

To turn it on, ts.Program instance must be created with ts.CompilerHost.jsDocParsingMode configuration like below. By the way, transformer libraries can't create the ts.Program instance by themselves, so if there're some transformer libraries utilizing the JSDocComment feature, they would be disabled after the TypeScript v5.3 release.

Especially, my library typia be damaged.

const options: ts.CompilerOptions;
const host: ts.CompilerHost = ts.createCompilerHost(options);
const program: ts.Program = ts.createProgram(["source files..."], options, host);

To support those JSDocComment utilizing transformers, ts-patch needs to hack the defaultJSDocParsingMode variable of below.

microsoft/TypeScript#55739 (comment) -> ts-patch author @nonara be tagged

Suggestion

I think there can be two options.

  • Just hack the defaultJSDocParsingMode value globally
  • Special CLI argument ts-patch install --jsDoc

The 1st way is to changing the defaultJSDocParsingMode variable of tsc to be 0 as default.

Comparing with other fast compilers like swc and esbuild, standard TypeScript compiler is hundreds of times slower. Therefore, as transformer library users are already enduring such slower compilation speed, I think they may not consider about the performance enhancement by skipping JsDocComment parsing.

The 2nd way is to providing special CLI argument ts-patch install --jsDoc.

Although saving compilation time by ignoring JsDocComment does not mean for transformer library users, it is the new formal spec of the standard TypeScript compiler. Therefore, rather than changing the defaultJSDocParsingMode variables globally, it may be more correct to give the user a choice.

@nonara
Copy link
Owner

nonara commented Nov 9, 2023

Thanks for the research, @samchon !

I think the best route would be to add a plugin config option which allows the user to specify jsdoc parsing mode.

If we have default be off, then plugin packages that rely on it will need to update their docs.

Conversely, if default is on, tspc will be lose speed optimizations by default.

I generally hate to put the burden of keeping up on the plugin maintainers, especially as some aren't maintained anymore. At the same time, it's difficult to justify slowing everyone else down by default.

I'm open to input on this from you or anyone else.

Right now, I think I'm inclined toward leaving the default to off and requiring an extra config option in the plugins entry.

@samchon
Copy link
Contributor Author

samchon commented Nov 9, 2023

Yes, the 2nd way would be suitable for the goal of TypeScript v5.3 update.

By the way, may I add a new idea about the 2nd way?

I'd thought about the ts-patch install --jsDoc option again, and it may be burden and annoying job for transformer library users. I think the configuration must be guided from transform library authors when setting them up, instead of the using global CLI argument, so that it would better to changing the tsconfig.json file like below.

{
  "compilerOptions": {
    "plugins": [
      { 
        "transform": "typia/lib/transform",
        "parseJSDoc": true // not a matter whatever the variable name be
      }
    ]
}

@nonara
Copy link
Owner

nonara commented Nov 9, 2023

Yes, that is what I was referring to here

I think the best route would be to add a plugin config option which allows the user to specify jsdoc parsing mode.

@samchon
Copy link
Contributor Author

samchon commented Nov 21, 2023

@nonara Ts v5.3 be released, and time to determine the spec.

samchon added a commit to samchon/ts-patch that referenced this issue Nov 21, 2023
As I don't know how @nonara determines about the `defaultJSDocParsingMode` and have no idea about the caching logic which can be compatible with before TS 5.2, just send a simple PR that changing `defaultJSDocParsingMode` variable value of `tsc.js` to be zero.

Hope @nonara to determine the `defaultJSDocParsingMode` strategy faster, and publish the new version of `ts-patch` for the TypeScript v5.3 update. Thanks for your amazing transformatoin ecosystem contributions.
@samchon
Copy link
Contributor Author

samchon commented Nov 24, 2023

Considering this spec, I think the cashing feature no more can maintainable, and just check the tsc.js and tsclibrary.js files manually whenever running the ts-patch install command. The good new is that, as ts-patch install command only takes a moment, and it would not be executed such a lot repeated, there would not be significant problem.

How do you think about @nonara ?

samchon added a commit to samchon/nestia that referenced this issue Nov 26, 2023
Since TypeScript v5.3 update, `tsc` no more parses `JSDocComment`s. Therefore, Therefore, `typia` and `@nestia/core` also cannot utilize those `JSDocComment` related features too, especially "Comment Tags" and "JSON schema generator".

However, in relation to this, the upgrade of `ts-patch` continues to be delayed, and I still don't know how long the delay would be continue. Furthermore, there're some `typia`/`nestia` users urging to resolve the `peerDependencies` of `typia`/`nestia` that blocking the TypeScript v5.3 update. Therefore, before the `ts-patch` being prepared, I've decoded to provide `typia`'s own solution for a while. It is the new CLI command `npx typia patch`, and `nestia` also adapts it (`npx nestia setup` command performs it).

Also, if the `defaultJSDocParsingMode` value not being patched, `typia` will generate an warning message of TypeScript compiler API. For reference, as it is an warning message, it does not interrupt the TypeScript compilation like the compilation error case. If there're some `typia`/`nestia` users never using "Comment Tags" or "JSON schema generator" at all, they don't need to run the CLI command. This is not mandatory command, but just optional command.

Of course, when `ts-patch` being updated, this CLI command would be disabled immediately, if the installed `ts-patch` version is the latest one.

Related issues:
  - samchon/typia#883
  - microsoft/TypeScript#55739
  - nonara/ts-patch#134
samchon added a commit to samchon/payments that referenced this issue Nov 28, 2023
@nonara
Copy link
Owner

nonara commented Dec 4, 2023

Ok. Doing this properly required a little bit of thought. I didn't like the option of universally overriding the default mode for tsc and losing the perf gains. The idea of forcing all end users to add something to their plugin entry config for libraries like typia was even worse.

I decided to take another step toward what we'll be doing in tsp-next and allow for basic configuration to be set within the actual plugin library itself.

Here's the working PR if you'd like to check it out: #138

I haven't it tested yet or added any tests. I'll try with typia tomorrow and ensure it works.

How It Works

What you'd be able to do here is specify configuration inside of typia's package.json file.

Something like this:

package.json

{
  // ...
  "tsp": {
    "tscOptions" : {
      "parseAllJsDoc": true
    }
  }
}

This will allow plugin authors to specify that they need full JSDoc parsing. The way it works now, it will preload the config for all plugins, and if any require that option, it will ensure that the host is updated to use the ParseAll mode before creating the program.

@samchon Feel free to let me know if you have any comments on that approach.

@samchon
Copy link
Contributor Author

samchon commented Dec 4, 2023

@nonara Good idea.

By the way, I think it would better to place into the tsconfig.json file. package.json file looks not fit for purpose.

For reference, regardless of which configuration file being targeted, whether tsconfig.json or package.json files, they use full names instead of abbreviations for property names. Also, looking at content of those tsconfig.json and package.json files, they preferred a simple list of variables rather than a deep hierarchy of classification systems.

In such reasons, I want to suggest such configurations into the tsconfig.json file.

In my case, I prefer 4th way.

1. TsConfig.compilerOptions.parseAllJsDoc

{
  "compilerOptions": {
    "parseAllJsDoc": true,
    "plugins": [],
  }
}

Until now, ttypescript and ts-patch had utilize the TsConfig.compilerOptions.plugins property for a long time, instead of making a new property for the transformation.

Therefore, the new property tsPatch (or tsp) can be sudden for ordinary users.

2. TsConfig.compilerOptions.tsPatch.parseAllJsDoc

{
  "compilerOptions": {
    "plugins": [],
    "tsPatch": {
      "parseAllJsDoc": true
    }
  }
}

As parseAllJsDoc is one of the compiler related feature, I think it should be placed into the compilerOptions property.

This way way would be suitable for purpose and meaning of the compilerOptions property.

3. TsConfig.tsPatch.parseAllJsDoc

{
  "compilerOptions": {
    "plugins": []
  },
  "ts-node": {},
  "ts-patch": {
    "parseAllJsDoc": true
  }
}

ts-node project guides to define the top level into tsconfig.json file like above.

If ts-patch also follows that way, many TypeScript may

4. TsConfig.compilerOptions.removeComments

{
  "compilerOptions": {
    "removeComments": false,
    "plugins": []
  }
}

Disable emitting comments.

If you've determined to turn on the parseAllJsDoc feature not by configuring individual plugins, but globally, I think the removeComments property of compilerOptions can be a good choice. This is a native property of tsconfig.json, and looking at its description, it is very suitable for the purpose.

In the previous issue, @nonara you told me that it would better to consider the legacy projects that had stopped maintaining. In my opinion, this way would be the best choice for them. This removeComments had been existed very along time ago, and by turning off that option. Furthermore, TypeScript user also can easily accomplish the compilation time reduction what TypeScript core team has wanted just by utilizing the native property.

@nonara
Copy link
Owner

nonara commented Dec 4, 2023

Sounds like there's a misunderstanding.

What I've done is made it so that you (the plugin author) can add some config on your plugin library's side, so that your users don't have to change anything.

In that case, you simply add the clause to your package.json. That will inform tsp that the plugin within your package requires full jsdoc during tsc.

In the plugin side, it wouldn't make sense to use tsconfig, as most libraries don't ship their tsconfig file, but all have a package file.

The way we're doing it is a common convention for this sort of thing, and it provides a place within the package file to define tsp specific detail.

Also – we definitely wouldn't add anything to compilerOptions or any other official tsconfig location. That's something I wish ttypescript had never done.

In the future, we'll be moving plugins to a special tsp section in tsconfig, but right now there is no change being made to user config.

@samchon
Copy link
Contributor Author

samchon commented Dec 4, 2023

Oh, I understood. It sounds amazing.

@nonara
Copy link
Owner

nonara commented Dec 5, 2023

Added to v3.1.0

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