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

Explicitly specify that TS 4.1 is required #485

Merged
merged 2 commits into from
Mar 5, 2021

Conversation

jstastny
Copy link
Contributor

@jstastny jstastny commented Mar 5, 2021

The compilation under TypeScript 4.0 fails.

$ npm install [email protected]
$ npm run validate:ts


> @octokit/[email protected] validate:ts /Users/honza/projects/webhooks.js
> tsc --noEmit --noImplicitAny --target es2020 --esModuleInterop --moduleResolution node test/typescript-validate.ts

src/types.ts:12:27 - error TS1110: Type expected.

12 > = TEmitterEvent extends `${infer TWebhookEvent}.${infer TAction}`
                             ~~~

src/types.ts:12:36 - error TS1005: '}' expected.

12 > = TEmitterEvent extends `${infer TWebhookEvent}.${infer TAction}`
                                      ~~~~~~~~~~~~~

src/types.ts:12:49 - error TS1128: Declaration or statement expected.

12 > = TEmitterEvent extends `${infer TWebhookEvent}.${infer TAction}`
                                                   ~

src/types.ts:12:50 - error TS1128: Declaration or statement expected.

12 > = TEmitterEvent extends `${infer TWebhookEvent}.${infer TAction}`
                                                    ~

src/types.ts:12:52 - error TS1005: ';' expected.

12 > = TEmitterEvent extends `${infer TWebhookEvent}.${infer TAction}`
                                                      ~

src/types.ts:12:59 - error TS1005: ';' expected.

12 > = TEmitterEvent extends `${infer TWebhookEvent}.${infer TAction}`
                                                             ~~~~~~~

src/types.ts:53:55 - error TS1005: ';' expected.

53  * Error object with optional properties coming from `octokit.request` errors
                                                         ~~~~~~~

src/types.ts:77:1 - error TS1160: Unterminated template literal.

77



Found 8 errors.

npm ERR! code ELIFECYCLE
npm ERR! errno 2
npm ERR! @octokit/[email protected] validate:ts: `tsc --noEmit --noImplicitAny --target es2020 --esModuleInterop --moduleResolution node test/typescript-validate.ts`
npm ERR! Exit status 2
npm ERR!
npm ERR! Failed at the @octokit/[email protected] validate:ts script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

I am not sure if the package-json.lock should be modified as well or not. I have modified it to match the minimum required version and therefore make the continuous integration run on that version, which is IMO an advantage.

gr2m
gr2m previously approved these changes Mar 5, 2021
@gr2m gr2m added Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR typescript Relevant to TypeScript users only labels Mar 5, 2021
@wolfy1339
Copy link
Member

package-lock.json currently (on master branch) contains version 4.2.2 of TypeScript, can you revert that change since it isn't necessary

@jstastny
Copy link
Contributor Author

jstastny commented Mar 5, 2021

package-lock.json currently (on master branch) contains version 4.2.2 of TypeScript, can you revert that change since it isn't necessary

Yeah. I've seen that. I was not sure if you wanted to keep that newer (4.2.2) version in the lock and effectively run the tests on something newer that the lowest supported (4.1), which could lead to similar situation as this PR is trying to fix -- the this package uses features that are not available in Typescript declared in package.json.

@wolfy1339
Copy link
Member

That is a good point.
That would be a follow up issue to program CI to test against the supported versions of TS

@jstastny
Copy link
Contributor Author

jstastny commented Mar 5, 2021

Meaning I should still revert the lock file in this PR, correct?

@wolfy1339
Copy link
Member

Yes, please do

The compilation under TypeScript 4.0 fails
@jstastny
Copy link
Contributor Author

jstastny commented Mar 5, 2021

Ok. Done.

@wolfy1339 wolfy1339 merged commit b5fdf2a into octokit:master Mar 5, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2021

🎉 This PR is included in version 8.5.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@oscard0m
Copy link
Member

oscard0m commented Mar 6, 2021

That is a good point.
That would be a follow up issue to program CI to test against the supported versions of TS

Created follow up issue: #486

This was referenced Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR typescript Relevant to TypeScript users only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants