chore: Prepare code for ESM #1921
Conversation
This prepares the code to compile properly in ESM to keep the code changes for full ESM later minimal. This does not convert Probot to ESM.
|
Not sure about the test failures, I'll still have to look at that (These seem to happen randomly) |
|
Oh, smee.io seems to be down, and that causes the tests to hang. @gr2m Are you maintaining this or is it someone else? |
Apparently, it works, but seems to reply incredibly slow for me. If you're maintaining it: Is it my internet connection (But actions seem to be affected too), DDoS, high load or another issue? |
|
Annoying. Currently refactoring the smee client. |
|
smee.io is a big headache, unfortunately 😭 GitHub now has webhooks via websockets in beta: https://github.blog/changelog/2022-11-16-webhook-forwarding-in-the-github-cli-public-beta/ and I built a library that made it possible to use it for GitHub App development: https://github.com/gr2m/github-app-webhook-relay/ My plan was to use with Probot and deprecate smee.io. However I recently learned that GitHub is considering to deprecate webhook forwarding due to low usage and high maintenance overhead. I'm not sure what to do. I don't know what the timeline is to deprecate it, if it will be replaced with something else, etc. But the situation with smee is not much better, and I think we should probably use the local webhook relay anyway. At least we can gather some user feedback that way to inform a decision within github what to do about them. An alternative would be to create a replacement service for smee.io 🤷🏼
|
|
AFAIK, GitHub themselves use smee.io in their automated tests (There is a certain smee.io endpoint hardcoded which is still getting requests), so I guess they also don't want that to be deprecated... |
|
I guess smee.io could be rewritten using faster frameworks like fastify and maybe having optimizations in redis, like using enableAutoPipelining. |
|
I refactored the smee client. I can also have a look in smee.io code and try to improve the performance. But i would only do it, if it has a chance to get merged. I somehow feel that improving smee.io could be rejected... |
|
I've inquired about the latest state of smee.io and will keep you posted. |
| import type { PushEvent } from "@octokit/webhooks-types"; | ||
|
|
||
| const pushEventPayload = ( | ||
| WebhookExamples.filter( |
There was a problem hiding this comment.
What's going on here? Doesn't typescript pick up the index.d.ts?
There was a problem hiding this comment.
It doesn't when ESM mode is enabled. Probably because the main file of @octokit/webhook-types is in JSON, and JSON imports in ESM aren't supported yet. They work at runtime, because Node.js has backwards-compatibility with non-ESM packages.
I assume this is a bug in TypeScript.
|
I personally am not convinced by ESM and that we have to name the import files to be .js suffixed. At fastify our modules are able to be required and imported. So i would wish we could do this for probot too. I am not blocking it, but I am not able to approve it, because I have avoided esm till now :))). But i am looking on this PR interest ;) |
There was a problem hiding this comment.
I think it's odd, too. I'm all in on ESM and hope we can conclude the transition of @octokit to ESM, but I'm not optimistic that this will happen anytime soon.
But I'm also not opposed to the change, if we change our preference, we can change it again, no big deal.
|
🎉 This PR is included in version 13.0.0-beta.12 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This prepares the code to compile properly in ESM to keep the code changes for full ESM later minimal. This does not convert Probot to ESM.