Skip to content

Conversation

@G-Rath
Copy link
Member

@G-Rath G-Rath commented Jan 8, 2021

This is branched off #252 to avoid me having to do an ugly rebase - ideally that should be merged first to make this review-able


Look, it's type generation!

Also ported over the code to generate the EventPayloadMap interface that can be used to do dynamic mapping.

I've made this a draft for now as while I think it's ready for review, it requires #252 and I think we should discuss how we're handling the build flow - I think it could benefit from a bit of a revision now that we've got a bit of a chain going.

In particular, do we want to check-in the schema.d.ts? Personally I think it would make reviewing easier, but we currently don't check-in the schema.json which is required by this script.

(this is currently written in TS, but can convert to JS easily if it looks like #254 isn't going to be merged for a bit)

@G-Rath G-Rath added the Type: Feature New feature or request label Jan 8, 2021
@G-Rath G-Rath changed the title Generate types feat: generate types from schema Jan 8, 2021
@G-Rath G-Rath marked this pull request as ready for review January 11, 2021 18:32
"@types/cheerio": "^0.22.23",
"@types/json-diff": "^0.5.0",
"@types/json-schema": "^7.0.6",
"@types/lodash": "^4.14.167",
Copy link
Member Author

Choose a reason for hiding this comment

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

This is needed as a fix until bcherny/json-schema-to-typescript#362 is landed.

@G-Rath
Copy link
Member Author

G-Rath commented Jan 11, 2021

can't statically import a file that's generated by a build command 🤦

Copy link
Contributor

@gr2m gr2m left a comment

Choose a reason for hiding this comment

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

I think eventually I'd move the event types into a different repository, because they are only relevant to the JavaScript Octokit projects, but for now we can add it here.

@gr2m
Copy link
Contributor

gr2m commented Jan 13, 2021

Could you add a section to the README on how to import the types? It would be good to show how to import the type for a specific event payload. That would answer @AlCalzone's question at probot/probot#1456

@G-Rath
Copy link
Member Author

G-Rath commented Jan 14, 2021

@gr2m done - I'll merge this shortly, and start on my refactor of webhooks.js & co 🎉

@G-Rath G-Rath merged commit 31931b6 into octokit:master Jan 14, 2021
@G-Rath G-Rath deleted the generate-types branch January 14, 2021 23:53
@octokitbot
Copy link
Contributor

🎉 This PR is included in version 3.35.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@gr2m
Copy link
Contributor

gr2m commented Jan 14, 2021

Great work everyone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants