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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow overriding tsconfig options from within tsconfig.json #1891

Merged
merged 1 commit into from Apr 9, 2022
Merged

Allow overriding tsconfig options from within tsconfig.json #1891

merged 1 commit into from Apr 9, 2022

Conversation

schlusslicht
Copy link
Contributor

@schlusslicht schlusslicht commented Mar 22, 2022

What this pull request contains

  • full lint/test compliance
  • implementation

What this pull request does
This pull request implements a new TypeDoc option: compilerOptions. By employing this option, one may selectively override some fields of the tsconfig.json used by TypeDoc.

An example
Let's take the following (trimmed-down) tsconfig.json:

{
  "compilerOptions": {
    // Enforce strictness
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "strict": true,
  },

  "typedocOptions": {
    // Abandon strictness
    "compilerOptions": {
      "noUnusedLocals": false,
      "noUnusedParameters": false,
      "strict": false
    }
  }
}

With this configuration, TypeScript strictness will be enforced throughout the codebase. But when running typedoc, the strictness will be dropped by patching the original compilerOptions with the provided typedocOptions.compilerOptions.

Motivation
When working with abstracted TypeScript tooling (e.g. not using tsc directly, but rollup-plugin-typescript2), the handling and transpilation of source code within a project may differ between those abstracted tools and tsc (and therefore TypeDoc). Allowing the user to override parts of the tsconfig.json for TypeDoc from within the same tsconfig.json seems to be a quite well scoped way to tackle these differences (without throwing yet another tsconfig.typedoc.json into the project and polluting the scene further).

Real world motivation
As an anecdotic example, I recently added a library to a project I'm currently working on and everything went well. Until I tried to build the documentation for said project. ... Because the libary distributes TypeScript source files instead of declaration files (.ts instead of .d.ts) the --skipLibCheck option of TypeScript does not catch those files and tsc keeps insisting on type-checking those sources within the node_modules folder, which throw errors. And as the library was built without having the TypeScript strict-mode enabled, I now cannot get tsc to run on my project anymore, while rollup-plugin-typescript2 happily transpiles my codebase and creates valid TypeScript definitions. To stress this, there are many, many examples of other people having the same issue and trying to get the TypeScript maintainers to implement a way around such behavior:

Meanwhile, Deno even implemented a CLI flag for exactly this.

Full disclosure
I know, I know, I could just create something like tsconfig.typedoc.json and manually override my desired preferences within this extending TypeScript configuration file. But, as stated above, I really don't like to clutter up my repository with "unnecessary" files. And to me it feels like a good practice to have the TypeScript configuration, the typedocOptions and the compilerOptions overrides all in one place: A single tsconfig.json! 馃槃

schlusslicht added a commit to sgrud/client that referenced this pull request Mar 22, 2022
@Gerrit0
Copy link
Collaborator

Gerrit0 commented Mar 23, 2022

I'm not entirely against this, though I'd say said library is bad.. shouldn't be shipping TS.

Putting compiler options in tsconfig seems ripe for confusion, I'm leaning towards the same key name used by ts-node, compilerOptions. https://typestrong.org/ts-node/docs/configuration#via-tsconfigjson-recommended

This should also be configurable within typedoc.json, which is another reason for using a separate name.

@schlusslicht
Copy link
Contributor Author

Thank You for Your reply and insights .. and I do concur. Furthermore, I've noticed, that the watch-mode uses another way of configuring TypeScript, which isn't covered by this pull request, so I'll go ahead and mark this pull request as draft.

I'm currently also investigating the possibility of implementing such overrides via a plugin, but this seems a little off, as configuration parsing should be (IMO) implemented within the core of an application.

Do You have any thoughts on this? 馃

@schlusslicht schlusslicht marked this pull request as draft March 23, 2022 13:36
@schlusslicht
Copy link
Contributor Author

schlusslicht commented Mar 23, 2022

I've looked through the sources again and I think I may have found a quite minimal, yet better way of implementing the discussed feature. Furthermore, I thought it might be desirable to let users also pass such overrides via CLI, so I went ahead and implemented JSON parsing, iff the compilerOptions parameter is passed as a string.

I also thought of implementing some test case(s) for this feature, but I wasn't sure where to put such a test case, as the options.test.ts only contains generic tests and the default-options.test.ts would be wrong place, too.

If You have any issues with the code style, logic or missing test cases, please let me know!

@schlusslicht schlusslicht marked this pull request as ready for review March 23, 2022 15:36
@Gerrit0
Copy link
Collaborator

Gerrit0 commented Mar 24, 2022

I think I'd rather not do the JSON parsing - it makes the logic more complicated and I don't see anyone ever using it. None of the other complex objects do it either.

(Also, if keeping it, the validate function isn't sufficient since it doesn't actually do checks on the parsed value)

Other than that, I like it! The default-options.ts file is actually the appropriate place to add tests - it's meant to test validation for our default options (e.g. those not added by plugins)

@schlusslicht
Copy link
Contributor Author

Hey,

sorry for not getting back to You sooner. As You suggested, I threw out the JSON parsing and kept the remaining changes as simple and small as possible. Regarding the test case, it's currently just a duplication of the markedOptions test case, so not much inspiration here. (I took ~10 minutes to skim the programmatic TS API and looked for an exposed function providing validation for passed-in compilerOptions, to no avail. Then again, the same should be done for the markedOptions - and it should actually be called in the validation hook instead of only as a test case. And after those thoughts I decided to keep it minimalistic and ask for Your opinion. 馃)

As always, if You have any issues with the code(style), please let me know and I'll try to address these.

Thank You and kind regards!

Copy link
Collaborator

@Gerrit0 Gerrit0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!

@Gerrit0 Gerrit0 merged commit 3cb756e into TypeStrong:master Apr 9, 2022
@schlusslicht schlusslicht deleted the feat/override-tsconfig branch April 10, 2022 10:28
schlusslicht added a commit to sgrud/client that referenced this pull request Apr 11, 2022
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

Successfully merging this pull request may close these issues.

None yet

2 participants