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

Fix missing ES types regression in 0.15 #100

Merged
merged 5 commits into from May 25, 2021

Conversation

bendrucker
Copy link
Contributor

@bendrucker bendrucker commented May 12, 2021

I noticed a few projects failing when Dependabot updated tsd from 0.14 to 0.15. Using git bisect I traced this to f5b69ed, specifically:

f5b69ed#diff-e717ee248865fa4044ef4561e4692693459eb15f9c1605b12ca42b8f38226fa3R37

Changing lib.es2017.d.ts to es2017 results in errors when typings use core ES types like RegExp, Array, and JSON (Cannot find name ${type}).

bendrucker/creditcards#43
bendrucker/creditcards-types#70
bendrucker/snakecase-keys#49

Presumably if any of these typings used DOM types, that would fail similarly. Digging further, it seemed suspicious that the test for adding DOM by default explicitly enables dom as an override:

f5b69ed#diff-ec66651dab3235538c4084a1f2f27a2a10d2752f03b28af09fbd18a16b4384baR5

If dom is enabled by default, the test fixture should not have to override. Turns out this was needed because otherwise findConfigFile traverses upwards until it finds a tsconfig.json file, so tests were inheriting the options used to build tsd itself by default.
In order to get test coverage more akin to end usage, I renamed the tsconfig file to exclude it from automatic discovery.

The resulting object from getOptionsFromTsConfig has lib mapped lib.${lib}.d.ts form. So without a tsconfig.json, the DOM test fails. If lib is transformed into full filenames, it passes.

Those library mappings come from here:

https://github.com/microsoft/TypeScript/blob/fe4a6709da77c7eb7cff1ba2a31963e18037bc86/src/compiler/commandLineParser.ts#L20

When working directly with a CompilerOptions, I can't find any public method that would transform the array of lib keys to their filenames. It should be safe to go with a mapping function since the only lib entries that don't obey that strict relationship is mutable targets like esnext, which don't seem like they would be specified in tsd.

I could also convert the compiler options back to their tsconfig.json representation and use convertCompilerOptionsFromJson instead.

Not invested in any particular fix here, just wanted to provide a thorough report. Happy to revise.

Closes #101

- extend default `dom` lib tests
- restore the missing lib test
Copy link
Collaborator

@BendingBender BendingBender left a comment

Choose a reason for hiding this comment

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

I've made a few improvements for your PR. Could you please apply the PR to your branch: bendrucker#1

Copy link
Collaborator

@BendingBender BendingBender left a comment

Choose a reason for hiding this comment

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

LGTM

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.

ts lib change broke existing usages
3 participants