Configure VS Code to use the local TypeScript version #728
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `typescript.tsdk` setting had inadvertently been moved inside the `[typescript]` block, which is not supported and thus meant it was ignored. This also enables the new `typescript.enablePromptUseWorkspaceTsdk` setting to prompt switching to the local TypeScript version (see https://code.visualstudio.com/updates/v1_45#_prompt-users-to-switch-to-the-workspace-version-of-typescript).
abernix
approved these changes
May 4, 2021
martijnwalraven
deleted the
martijnwalraven/consistent-vscode-typescript-version
branch
glasser
added a commit
to apollographql/apollo-server
that referenced
this pull request
May 4, 2021
Upgrading to TS 4 is generally useful, but it also lets use combine `composite` with `noEmit`, which lets us actually use `tsconfig.test.json`. This PR copies a few things we've done recently in the Federation repo. - Upgrade TypeScript, Jest, and Codecov. - Drop jest-cucumber dep (hasn't been used for a while). - Update VSCode task to use the "typescript" type. - Set enablePromptUseWorkspaceTsdk like in apollographql/federation#728 - Stop using moduleNameMapper in jest config, which seemed to create weird issues. Instead, just make sure that `npm run compile` happens before you run tests (at least when run via `npm test`) via a `pretest` script. - Fix a lot of compilation errors (mostly caused by actually typechecking tests, though some from the upgrade). Most of this is pretty localized, but some required more work (eg, replacing a `Map` that was being treated as a `KeyValueCache` with an actual `KeyValueCache` implementation). - I found the moduleNameMapper pulling in a top-level `__mocks__` to be confusing and also led to errors about trying to import files from outside a project. In fact in general I found `__mocks__` to be confusing. So: - I replaced our use of a custom `Date` mocker plus `jest.useFakeTimers` with `@sinonjs/fake-timers` (which does both and is what jest's is built on) - Replaced `__mocks__/apollo-server-env.ts` with a file that doesn't have a special `__mocks__` name - Did the same for the ioredis mock, and loaded it into `jest.mock` explicitly rather than implicitly - The thing where we imported a function that defined a jest test suite from the `__tests__` of `apollo-server-caching` was sort of weird already, and importing from a project that used `noEmit` didn't seem to work at all here. So I changed the testing function to just be a normal non-Jest-specific function exported from `apollo-server-caching` that can be used in Jest tests (or non-Jest tests for that matter) with a bit of preamble. (I changed its name too so it's obviously a different function.) Because closing the cache is now not part of the test, we didn't need `TestableKeyValueCache` any more so I dropped it. - isDirectiveDefined isn't exported and is now always called with an array, so remove code that runs if it doesn't get one and tests that that code works. - RESTDataSource used to document and test that you can override `baseURL` in a subclass with a getter. This is illegal in TS4 so we don't officially support it any more. Additionally, this PR removes global types from `apollo-server-env` (the `global.d.ts` file). Thanks to @martijnwalraven for figuring this out and writing the following: We use exports from `apollo-server-env` to access the Fetch API to avoid depending on running in specific environments. While environments like Cloudflare Workers and Deno expose a Fetch API globally, in Node environments you'd generally like to avoid polyfilling the global context. There are also differences between the APIs available in these environments that we may want to account for with our own types. Unfortunately,`apollo-fetch` uses `cross-fetch` as a global polyfill (see https://github.com/apollographql/apollo-fetch/blob/0a0b8df241960dfa72abf3bd179e9d936265a7da/packages/apollo-fetch/src/apollo-fetch.ts#L17), and expects having Fetch API types defined globally. Because we use `apollo-fetch` in our tests, we've long used a `global.d.ts` in `apollo-server-env` to make the exported types available globally without depending on the `dom` lib types (because most of the types in there don't really make sense in non-browser environments and can lead to accidental usage). As it turns out however, we also use `superagent` in our tests, and `@types/superagent` now specifies an explicit dependency on `dom` lib. As TypeScript makes any global type declarations available throughout a project, that means our tests now get the `dom` lib types merged into their environment, resulting in conflicts with the types in `apollo-server-env`'s `global.d.ts`. Forcing anyone depending on packages like `@types/superagent` to have their environment extended with the `dom` lib types isn't great, and there are existing discussions about the implications of this for that package specifically (see DefinitelyTyped/DefinitelyTyped#41425). There is also a promising proposal for a `strictEnvironment` option in TypeScript that would help avoid these situations (see https://gist.github.com/RyanCavanaugh/702ebd1ca2fc060e58e634b4e30c1c1c). For now, I've solved this by reluctantly adding `dom` to `tsconfig.test.base.json`. We should be able to remove this when our dependencies get fixed or if `strictEnvironment` gets implemented. I also fixed some typing issues that ironically illustrate the danger of having these `dom` types available. Because the mocked version of `apollo-server-env` exported classes through a `const` declaration, that didn't actually export them as types, just as values (see microsoft/TypeScript#36348). But because we have similarly named types defined in `dom`, those were used instead. Using named exports in the `apollo-server-env` mock avoids this.
glasser
added a commit
to apollographql/apollo-server
that referenced
this pull request
May 4, 2021
Upgrading to TS 4 is generally useful, but it also lets use combine `composite` with `noEmit`, which lets us actually use `tsconfig.test.json`. This PR copies a few things we've done recently in the Federation repo. - Upgrade TypeScript, Jest, and Codecov. - Drop jest-cucumber dep (hasn't been used for a while). - Update VSCode task to use the "typescript" type. - Set enablePromptUseWorkspaceTsdk like in apollographql/federation#728 - Stop using moduleNameMapper in jest config, which seemed to create weird issues. Instead, just make sure that `npm run compile` happens before you run tests (at least when run via `npm test`) via a `pretest` script. - Fix a lot of compilation errors (mostly caused by actually typechecking tests, though some from the upgrade). Most of this is pretty localized, but some required more work (eg, replacing a `Map` that was being treated as a `KeyValueCache` with an actual `KeyValueCache` implementation). - I found the moduleNameMapper pulling in a top-level `__mocks__` to be confusing and also led to errors about trying to import files from outside a project. In fact in general I found `__mocks__` to be confusing. So: - I replaced our use of a custom `Date` mocker plus `jest.useFakeTimers` with `@sinonjs/fake-timers` (which does both and is what jest's is built on) - Replaced `__mocks__/apollo-server-env.ts` with a file that doesn't have a special `__mocks__` name - Did the same for the ioredis mock, and loaded it into `jest.mock` explicitly rather than implicitly - The thing where we imported a function that defined a jest test suite from the `__tests__` of `apollo-server-caching` was sort of weird already, and importing from a project that used `noEmit` didn't seem to work at all here. So I changed the testing function to just be a normal non-Jest-specific function exported from `apollo-server-caching` that can be used in Jest tests (or non-Jest tests for that matter) with a bit of preamble. (I changed its name too so it's obviously a different function.) Because closing the cache is now not part of the test, we didn't need `TestableKeyValueCache` any more so I dropped it. - isDirectiveDefined isn't exported and is now always called with an array, so remove code that runs if it doesn't get one and tests that that code works. - RESTDataSource used to document and test that you can override `baseURL` in a subclass with a getter. This is illegal in TS4 so we don't officially support it any more. Additionally, this PR removes global types from `apollo-server-env` (the `global.d.ts` file). Thanks to @martijnwalraven for figuring this out and writing the following: We use exports from `apollo-server-env` to access the Fetch API to avoid depending on running in specific environments. While environments like Cloudflare Workers and Deno expose a Fetch API globally, in Node environments you'd generally like to avoid polyfilling the global context. There are also differences between the APIs available in these environments that we may want to account for with our own types. Unfortunately,`apollo-fetch` uses `cross-fetch` as a global polyfill (see https://github.com/apollographql/apollo-fetch/blob/0a0b8df241960dfa72abf3bd179e9d936265a7da/packages/apollo-fetch/src/apollo-fetch.ts#L17), and expects having Fetch API types defined globally. Because we use `apollo-fetch` in our tests, we've long used a `global.d.ts` in `apollo-server-env` to make the exported types available globally without depending on the `dom` lib types (because most of the types in there don't really make sense in non-browser environments and can lead to accidental usage). As it turns out however, we also use `superagent` in our tests, and `@types/superagent` now specifies an explicit dependency on `dom` lib. As TypeScript makes any global type declarations available throughout a project, that means our tests now get the `dom` lib types merged into their environment, resulting in conflicts with the types in `apollo-server-env`'s `global.d.ts`. Forcing anyone depending on packages like `@types/superagent` to have their environment extended with the `dom` lib types isn't great, and there are existing discussions about the implications of this for that package specifically (see DefinitelyTyped/DefinitelyTyped#41425). There is also a promising proposal for a `strictEnvironment` option in TypeScript that would help avoid these situations (see https://gist.github.com/RyanCavanaugh/702ebd1ca2fc060e58e634b4e30c1c1c). For now, I've solved this by reluctantly adding `dom` to `tsconfig.test.base.json`. We should be able to remove this when our dependencies get fixed or if `strictEnvironment` gets implemented. I also fixed some typing issues that ironically illustrate the danger of having these `dom` types available. Because the mocked version of `apollo-server-env` exported classes through a `const` declaration, that didn't actually export them as types, just as values (see microsoft/TypeScript#36348). But because we have similarly named types defined in `dom`, those were used instead. Using named exports in the `apollo-server-env` mock avoids this. Fixes #5119.
glasser
added a commit
to apollographql/apollo-server
that referenced
this pull request
May 4, 2021
Upgrading to TS 4 is generally useful, but it also lets use combine `composite` with `noEmit`, which lets us actually use `tsconfig.test.json`. This PR copies a few things we've done recently in the Federation repo. - Upgrade TypeScript, Jest, and Codecov. - Drop jest-cucumber dep (hasn't been used for a while). - Update VSCode task to use the "typescript" type. - Set enablePromptUseWorkspaceTsdk like in apollographql/federation#728 - Stop using moduleNameMapper in jest config, which seemed to create weird issues. Instead, just make sure that `npm run compile` happens before you run tests (at least when run via `npm test`) via a `pretest` script. - Fix a lot of compilation errors (mostly caused by actually typechecking tests, though some from the upgrade). Most of this is pretty localized, but some required more work (eg, replacing a `Map` that was being treated as a `KeyValueCache` with an actual `KeyValueCache` implementation). - I found the moduleNameMapper pulling in a top-level `__mocks__` to be confusing and also led to errors about trying to import files from outside a project. In fact in general I found `__mocks__` to be confusing. So: - I replaced our use of a custom `Date` mocker plus `jest.useFakeTimers` with `@sinonjs/fake-timers` (which does both and is what jest's is built on) - Replaced `__mocks__/apollo-server-env.ts` with a file that doesn't have a special `__mocks__` name - Did the same for the ioredis mock, and loaded it into `jest.mock` explicitly rather than implicitly - The thing where we imported a function that defined a jest test suite from the `__tests__` of `apollo-server-caching` was sort of weird already, and importing from a project that used `noEmit` didn't seem to work at all here. So I changed the testing function to just be a normal non-Jest-specific function exported from `apollo-server-caching` that can be used in Jest tests (or non-Jest tests for that matter) with a bit of preamble. (I changed its name too so it's obviously a different function.) Because closing the cache is now not part of the test, we didn't need `TestableKeyValueCache` any more so I dropped it. - isDirectiveDefined isn't exported and is now always called with an array, so remove code that runs if it doesn't get one and tests that that code works. - RESTDataSource used to document and test that you can override `baseURL` in a subclass with a getter. This is illegal in TS4 so we don't officially support it any more. Additionally, this PR removes global types from `apollo-server-env` (the `global.d.ts` file). Thanks to @martijnwalraven for figuring this out and writing the following: We use exports from `apollo-server-env` to access the Fetch API to avoid depending on running in specific environments. While environments like Cloudflare Workers and Deno expose a Fetch API globally, in Node environments you'd generally like to avoid polyfilling the global context. There are also differences between the APIs available in these environments that we may want to account for with our own types. Unfortunately,`apollo-fetch` uses `cross-fetch` as a global polyfill (see https://github.com/apollographql/apollo-fetch/blob/0a0b8df241960dfa72abf3bd179e9d936265a7da/packages/apollo-fetch/src/apollo-fetch.ts#L17), and expects having Fetch API types defined globally. Because we use `apollo-fetch` in our tests, we've long used a `global.d.ts` in `apollo-server-env` to make the exported types available globally without depending on the `dom` lib types (because most of the types in there don't really make sense in non-browser environments and can lead to accidental usage). As it turns out however, we also use `superagent` in our tests, and `@types/superagent` now specifies an explicit dependency on `dom` lib. As TypeScript makes any global type declarations available throughout a project, that means our tests now get the `dom` lib types merged into their environment, resulting in conflicts with the types in `apollo-server-env`'s `global.d.ts`. Forcing anyone depending on packages like `@types/superagent` to have their environment extended with the `dom` lib types isn't great, and there are existing discussions about the implications of this for that package specifically (see DefinitelyTyped/DefinitelyTyped#41425). There is also a promising proposal for a `strictEnvironment` option in TypeScript that would help avoid these situations (see https://gist.github.com/RyanCavanaugh/702ebd1ca2fc060e58e634b4e30c1c1c). For now, I've solved this by reluctantly adding `dom` to `tsconfig.test.base.json`. We should be able to remove this when our dependencies get fixed or if `strictEnvironment` gets implemented. I also fixed some typing issues that ironically illustrate the danger of having these `dom` types available. Because the mocked version of `apollo-server-env` exported classes through a `const` declaration, that didn't actually export them as types, just as values (see microsoft/TypeScript#36348). But because we have similarly named types defined in `dom`, those were used instead. Using named exports in the `apollo-server-env` mock avoids this. Fixes #5119.
glasser
added a commit
to apollographql/apollo-server
that referenced
this pull request
May 5, 2021
Upgrading to TS 4 is generally useful, but it also lets use combine `composite` with `noEmit`, which lets us actually use `tsconfig.test.json`. This PR copies a few things we've done recently in the Federation repo. - Upgrade TypeScript, Jest, and Codecov. - Drop jest-cucumber dep (hasn't been used for a while). - Update VSCode task to use the "typescript" type. - Set enablePromptUseWorkspaceTsdk like in apollographql/federation#728 - Stop using moduleNameMapper in jest config, which seemed to create weird issues. Instead, just make sure that `npm run compile` happens before you run tests (at least when run via `npm test`) via a `pretest` script. - Fix a lot of compilation errors (mostly caused by actually typechecking tests, though some from the upgrade). Most of this is pretty localized, but some required more work (eg, replacing a `Map` that was being treated as a `KeyValueCache` with an actual `KeyValueCache` implementation). - I found the moduleNameMapper pulling in a top-level `__mocks__` to be confusing and also led to errors about trying to import files from outside a project. In fact in general I found `__mocks__` to be confusing. So: - I replaced our use of a custom `Date` mocker plus `jest.useFakeTimers` with `@sinonjs/fake-timers` (which does both and is what jest's is built on) - Replaced `__mocks__/apollo-server-env.ts` with a file that doesn't have a special `__mocks__` name - Did the same for the ioredis mock, and loaded it into `jest.mock` explicitly rather than implicitly - The thing where we imported a function that defined a jest test suite from the `__tests__` of `apollo-server-caching` was sort of weird already, and importing from a project that used `noEmit` didn't seem to work at all here. So I changed the testing function to just be a normal non-Jest-specific function exported from `apollo-server-caching` that can be used in Jest tests (or non-Jest tests for that matter) with a bit of preamble. (I changed its name too so it's obviously a different function.) Because closing the cache is now not part of the test, we didn't need `TestableKeyValueCache` any more so I dropped it. - isDirectiveDefined isn't exported and is now always called with an array, so remove code that runs if it doesn't get one and tests that that code works. - RESTDataSource used to document and test that you can override `baseURL` in a subclass with a getter. This is illegal in TS4 so we don't officially support it any more. Additionally, this PR removes global types from `apollo-server-env` (the `global.d.ts` file). Thanks to @martijnwalraven for figuring this out and writing the following: We use exports from `apollo-server-env` to access the Fetch API to avoid depending on running in specific environments. While environments like Cloudflare Workers and Deno expose a Fetch API globally, in Node environments you'd generally like to avoid polyfilling the global context. There are also differences between the APIs available in these environments that we may want to account for with our own types. Unfortunately,`apollo-fetch` uses `cross-fetch` as a global polyfill (see https://github.com/apollographql/apollo-fetch/blob/0a0b8df241960dfa72abf3bd179e9d936265a7da/packages/apollo-fetch/src/apollo-fetch.ts#L17), and expects having Fetch API types defined globally. Because we use `apollo-fetch` in our tests, we've long used a `global.d.ts` in `apollo-server-env` to make the exported types available globally without depending on the `dom` lib types (because most of the types in there don't really make sense in non-browser environments and can lead to accidental usage). As it turns out however, we also use `superagent` in our tests, and `@types/superagent` now specifies an explicit dependency on `dom` lib. As TypeScript makes any global type declarations available throughout a project, that means our tests now get the `dom` lib types merged into their environment, resulting in conflicts with the types in `apollo-server-env`'s `global.d.ts`. Forcing anyone depending on packages like `@types/superagent` to have their environment extended with the `dom` lib types isn't great, and there are existing discussions about the implications of this for that package specifically (see DefinitelyTyped/DefinitelyTyped#41425). There is also a promising proposal for a `strictEnvironment` option in TypeScript that would help avoid these situations (see https://gist.github.com/RyanCavanaugh/702ebd1ca2fc060e58e634b4e30c1c1c). For now, I've solved this by reluctantly adding `dom` to `tsconfig.test.base.json`. We should be able to remove this when our dependencies get fixed or if `strictEnvironment` gets implemented. I also fixed some typing issues that ironically illustrate the danger of having these `dom` types available. Because the mocked version of `apollo-server-env` exported classes through a `const` declaration, that didn't actually export them as types, just as values (see microsoft/TypeScript#36348). But because we have similarly named types defined in `dom`, those were used instead. Using named exports in the `apollo-server-env` mock avoids this. Fixes #5119.
trevor-scheer
pushed a commit
to apollographql/apollo-datasource
that referenced
this pull request
Feb 1, 2022
Upgrading to TS 4 is generally useful, but it also lets use combine `composite` with `noEmit`, which lets us actually use `tsconfig.test.json`. This PR copies a few things we've done recently in the Federation repo. - Upgrade TypeScript, Jest, and Codecov. - Drop jest-cucumber dep (hasn't been used for a while). - Update VSCode task to use the "typescript" type. - Set enablePromptUseWorkspaceTsdk like in apollographql/federation#728 - Stop using moduleNameMapper in jest config, which seemed to create weird issues. Instead, just make sure that `npm run compile` happens before you run tests (at least when run via `npm test`) via a `pretest` script. - Fix a lot of compilation errors (mostly caused by actually typechecking tests, though some from the upgrade). Most of this is pretty localized, but some required more work (eg, replacing a `Map` that was being treated as a `KeyValueCache` with an actual `KeyValueCache` implementation). - I found the moduleNameMapper pulling in a top-level `__mocks__` to be confusing and also led to errors about trying to import files from outside a project. In fact in general I found `__mocks__` to be confusing. So: - I replaced our use of a custom `Date` mocker plus `jest.useFakeTimers` with `@sinonjs/fake-timers` (which does both and is what jest's is built on) - Replaced `__mocks__/apollo-server-env.ts` with a file that doesn't have a special `__mocks__` name - Did the same for the ioredis mock, and loaded it into `jest.mock` explicitly rather than implicitly - The thing where we imported a function that defined a jest test suite from the `__tests__` of `apollo-server-caching` was sort of weird already, and importing from a project that used `noEmit` didn't seem to work at all here. So I changed the testing function to just be a normal non-Jest-specific function exported from `apollo-server-caching` that can be used in Jest tests (or non-Jest tests for that matter) with a bit of preamble. (I changed its name too so it's obviously a different function.) Because closing the cache is now not part of the test, we didn't need `TestableKeyValueCache` any more so I dropped it. - isDirectiveDefined isn't exported and is now always called with an array, so remove code that runs if it doesn't get one and tests that that code works. - RESTDataSource used to document and test that you can override `baseURL` in a subclass with a getter. This is illegal in TS4 so we don't officially support it any more. Additionally, this PR removes global types from `apollo-server-env` (the `global.d.ts` file). Thanks to @martijnwalraven for figuring this out and writing the following: We use exports from `apollo-server-env` to access the Fetch API to avoid depending on running in specific environments. While environments like Cloudflare Workers and Deno expose a Fetch API globally, in Node environments you'd generally like to avoid polyfilling the global context. There are also differences between the APIs available in these environments that we may want to account for with our own types. Unfortunately,`apollo-fetch` uses `cross-fetch` as a global polyfill (see https://github.com/apollographql/apollo-fetch/blob/0a0b8df241960dfa72abf3bd179e9d936265a7da/packages/apollo-fetch/src/apollo-fetch.ts#L17), and expects having Fetch API types defined globally. Because we use `apollo-fetch` in our tests, we've long used a `global.d.ts` in `apollo-server-env` to make the exported types available globally without depending on the `dom` lib types (because most of the types in there don't really make sense in non-browser environments and can lead to accidental usage). As it turns out however, we also use `superagent` in our tests, and `@types/superagent` now specifies an explicit dependency on `dom` lib. As TypeScript makes any global type declarations available throughout a project, that means our tests now get the `dom` lib types merged into their environment, resulting in conflicts with the types in `apollo-server-env`'s `global.d.ts`. Forcing anyone depending on packages like `@types/superagent` to have their environment extended with the `dom` lib types isn't great, and there are existing discussions about the implications of this for that package specifically (see DefinitelyTyped/DefinitelyTyped#41425). There is also a promising proposal for a `strictEnvironment` option in TypeScript that would help avoid these situations (see https://gist.github.com/RyanCavanaugh/702ebd1ca2fc060e58e634b4e30c1c1c). For now, I've solved this by reluctantly adding `dom` to `tsconfig.test.base.json`. We should be able to remove this when our dependencies get fixed or if `strictEnvironment` gets implemented. I also fixed some typing issues that ironically illustrate the danger of having these `dom` types available. Because the mocked version of `apollo-server-env` exported classes through a `const` declaration, that didn't actually export them as types, just as values (see microsoft/TypeScript#36348). But because we have similarly named types defined in `dom`, those were used instead. Using named exports in the `apollo-server-env` mock avoids this. Fixes #5119.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
typescript.tsdksetting had inadvertently been moved inside the[typescript]block, which is not supported and thus meant it was ignored.This also enables the new
typescript.enablePromptUseWorkspaceTsdksetting to prompt switching to the local TypeScript version (see https://code.visualstudio.com/updates/v1_45#_prompt-users-to-switch-to-the-workspace-version-of-typescript).