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

Add default tsconfig.json #12

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

skulptur
Copy link
Contributor

This uses the default tsconfig.json generated with tsc --init (tsc Version 3.9.5).

Only change from that right now is adding
"importsNotUsedAsValues": "error"

Copy link
Member

@ThaNarie ThaNarie left a comment

Choose a reason for hiding this comment

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

Besides the comments, everything else looks good to me:)

tsconfig.json Outdated Show resolved Hide resolved
tsconfig.json Outdated Show resolved Hide resolved
// "incremental": true, /* Enable incremental compilation */
"target": "es5", /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017', 'ES2018', 'ES2019', 'ES2020', or 'ESNEXT'. */
"module": "commonjs", /* Specify module code generation: 'none', 'commonjs', 'amd', 'system', 'umd', 'es2015', 'es2020', or 'ESNext'. */
// "lib": [], /* Specify library files to be included in the compilation. */
Copy link
Member

@ThaNarie ThaNarie Jun 18, 2020

Choose a reason for hiding this comment

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

Should we include some default libs here, or manage this in our boilerplates? e.g.:

"esnext",
"dom"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably makes sense to have some defaults here. I can imagine not using "dom" when writing libraries, but does having it anyway have any downside?

Copy link
Member

Choose a reason for hiding this comment

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

So we have 2 options:

  • leave this default, and require projects/boilerplates to set this explicitly (opt-in)
  • include esnext, dom to be future-compatible for most of our use cases, and allow projects to change or opt-out of this setting (opt-out)

Including more by default can only do harm of a project doesn't include all the polyfills for the given version, and developer uses it without testing their code.
But this could also happen if we include es2019 if we need some of the features, but haven't included polyfills for all of them.

Using esnext here means we don't have to update it every year, and kan keep using the newest features combined with babel-preset-env in our projects.

Choose a reason for hiding this comment

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

I like the 'opt-out' option, but I would suggest the following list:

      "ESNext",
      "DOM",
      "DOM.Iterable",
      "ScriptHost",
      "ESNext.AsyncIterable"

tsconfig.json Show resolved Hide resolved
// "paths": {}, /* A series of entries which re-map imports to lookup locations relative to the 'baseUrl'. */
// "rootDirs": [], /* List of root folders whose combined content represents the structure of the project at runtime. */
// "typeRoots": [], /* List of folders to include type definitions from. */
// "types": [], /* Type declaration files to be included in compilation. */
Copy link
Member

Choose a reason for hiding this comment

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

More often than not, I have to explicitly specify types here, since the default will include everything in the @types module folder.

Including everything often gives conflicts due to packages (mostly build-tools) having other types installed as sub-dependencies, that rely on the node environment instead of the web environment.

Not sure if there is something we can do in this file, but maybe some documentation about this common use case could be beneficial.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can put this file in a directory with a default type declaration file, and a .md for documentation if needed

Copy link
Member

@ThaNarie ThaNarie Jun 19, 2020

Choose a reason for hiding this comment

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

We can put this file in a directory with a default type declaration file

What do you mean by this? :)

Copy link
Contributor Author

@skulptur skulptur Jun 21, 2020

Choose a reason for hiding this comment

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

That quote I thought you meant adding a .d.ts file, but now I see it's about adding a list that would be tied to a project's dependencies. Correct?

That issue is annoying and I didn't know manually including stuff was the solution. So is there any other way between types and typeRoots to prevent that? Maybe if we identify which common libs we use tend to screw things up that would be a middle ground? Or is this a declare everything vs include everything kind of deal?

@ThaNarie ThaNarie mentioned this pull request Oct 27, 2020
@ThijsTyZ ThijsTyZ changed the base branch from master to main November 12, 2020 14:13
@psimk
Copy link

psimk commented Oct 11, 2021

I think that we don't need a default tsconfig.json, as it will have to be updated either whenever TS decides to add something or when node upgrades and we need to support a new module or target.

So I would much rather have tsconfig.json's defined in our skeletons, where we can change them when we decided to bump our Node or Typescript versions

@ThaNarie
Copy link
Member

So I would much rather have tsconfig.json's defined in our skeletons, where we can change them when we decided to bump our Node or Typescript versions

That's something that should definitely happen.

I think that we don't need a default tsconfig.json, as it will have to be updated either whenever TS decides to add something or when node upgrades and we need to support a new module or target.

I think the main reason for this is "documentation" purposes (and) as a starting point for any other project/library/skeleton that we create.

Maybe instead of a file, we could make it a markdown page, that includes:

  • The TS version this is last updated for
  • The configs where we explicitly want a certain value, either because of or coding standards / linting (e.g. strict mode, or how we import types)
  • And some configs that we set or change most of the times to make them work for certain project types (e.g. moduleResolution, types, etc).

And for the bottom 2, an explanation for why we dictate or advise these settings, to serve as documentation.

@psimk @skulptur how do you feel about that?

@ThijsTyZ
Copy link

Do we still need this PR? The tsconfig.json is now part of the Skeletons I assume

@psimk
Copy link

psimk commented Feb 21, 2022

@ThijsTyZ
This PR no, but we probably want to turn @ThaNarie's suggestions into an issue & then PR.

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.

4 participants