Skip to content
This repository has been archived by the owner on Nov 16, 2023. It is now read-only.

Sync README tsconfig.json with dts-gen template #345

Merged
merged 1 commit into from
Sep 7, 2021

Conversation

jablko
Copy link
Contributor

@jablko jablko commented Aug 31, 2021

Does it make sense to synchronize the tsconfig.json prescribed in the README with the dts-gen template? https://github.com/microsoft/dts-gen/blob/af84657554e01fcfa81b210a43efd8236f476fd4/lib/definitely-typed.ts#L100-L110

This PR adds "types": [] and "forceConsistentCasingInFileNames: true to the README. Similar to #77 and #122 although the current example probably only actually errors on DefinitelyTyped, not if you're running dtslint on some other library:

dtslint/src/checks.ts

Lines 61 to 71 in 33eb773

if (dt) {
const { relativeBaseUrl } = dt;
const mustHave = {
module: "commonjs",
noEmit: true,
forceConsistentCasingInFileNames: true,
baseUrl: relativeBaseUrl,
typeRoots: [relativeBaseUrl],
types: [],
};

Still, I maybe ran into this here -- the tsconfig.json in that case might have been based on the dtslint README, and probably most libraries want "types": [], to avoid implicitly including and linting every node_modules/@types declaration (in a monorepo with a bunch of dependencies that can be a lot of checks, multiplied by the number of linted workspaces).

@jablko jablko requested a review from sandersn as a code owner August 31, 2021 16:09
@sandersn
Copy link
Member

sandersn commented Sep 3, 2021

I checked dts-gen's history and it looks like the dts-gen template is 5 years old. Probably it makes sense to start with what dtslint allows and change dts-gen if needed.

However, isn't that true of the current state of the PR? It looks like every package on DT already includes types: [] and forceConsistentCasingInFileNames: true. So it's likely that this PR is correct as-is.

@jablko
Copy link
Contributor Author

jablko commented Sep 3, 2021

I checked dts-gen's history and it looks like the dts-gen template is 5 years old. Probably it makes sense to start with what dtslint allows and change dts-gen if needed.

When creating a DT package, you want to start with what dtslint allows (if I understand right). dts-gen's template satisfies dtslint's checks but the dtslint README doesn't -- so if you copy/paste from the README to a DT package, instead of using dts-gen, the DT CI will complain. This PR addresses the README.

dtslint does fewer checks on non-DT libraries -- e.g. it doesn't enforce "types": [] in that case. Still I think "types": [] specifically (and the dts-gen template generally) is a reasonable default in those cases as well, for the reason I encountered (linting every node_modules/@types declaration).

dts-gen generates no tsconfig.json for non-DT libraries (.d.ts file only). I hadn't considered whether, for non-DT libraries, 1) dtslint should enforce "types": [] or 2) dts-gen should generate a tsconfig.json. I only noticed the inadequate advice in the README.

However, isn't that true of the current state of the PR? It looks like every package on DT already includes types: [] and forceConsistentCasingInFileNames: true. So it's likely that this PR is correct as-is.

Yup, every package on DT includes those -- the DT CI will complain otherwise. dts-gen's template includes them too -- only the README and some non-DT libraries don't.

Did I get what you were saying ... or miss the point?

@sandersn
Copy link
Member

sandersn commented Sep 7, 2021

Yes, that explanation helps a lot.

@sandersn sandersn merged commit e28f894 into microsoft:master Sep 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants