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

feat: create SetWebhookUtil component #523

Closed
wants to merge 19 commits into from
Closed

Conversation

roziscoding
Copy link
Contributor

@roziscoding roziscoding commented Oct 9, 2022

Preview:

image
image
image
image

Closes #517

@KnorpelSenf
Copy link
Member

Would you be able to defer loading everything until hydration kicks in, using a dynamic import? I'm not sure it's a good idea to bundle the component framework and grammY and everything else into the static site, this will add a few hundred kB to the build.

The component is usually at the very bottom of the page, so in 99 % of page visits, we have a few minutes to download the extra weight and render the component.

Yes, this will mean that the component doesn't load if JS is disabled, but this actually makes a lot of sense anyway.

@roziscoding roziscoding marked this pull request as ready for review October 17, 2022 15:37
@roziscoding
Copy link
Contributor Author

roziscoding commented Oct 17, 2022

This does not target bullet points 2 and 5 from #517 yet, but is generally ready for review. I'll cover those points later this week.

Translation files:

  • es-ES: site/docs/.vuepress/components/set-webhook-util/translations/es-ES.ts
  • id: site/docs/.vuepress/components/set-webhook-util/translations/id.ts
  • zh-CN: site/docs/.vuepress/components/set-webhook-util/translations/zh-CN.ts

Localized Pages:

  • es-ES: site/docs/es/webhook-util.md
  • id: site/docs/id/webhook-util.md
  • zh-CN: site/docs/zh/webhook-util.md

@roziscoding
Copy link
Contributor Author

All features are now implemented.
The build fails locally because I didn't use the CDN version of grammY. I'll need some help getting that done.
Once I can build a production version, I can test possible performance improvements.

@KnorpelSenf
Copy link
Member

It looks like it OOMs?

@KnorpelSenf
Copy link
Member

I can take a closer look at a later point

@roziscoding
Copy link
Contributor Author

It looks like it OOMs?

Yeah, in github actions it fails due to OOM, but locally the error is different.
I'll post it here in about an hour when I get to the computer.

@rojvv rojvv mentioned this pull request Nov 21, 2022
Copy link
Member

@KnorpelSenf KnorpelSenf left a comment

Choose a reason for hiding this comment

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

Nice to finally see this unblocked 🤓
Ping me when I should review

@roziscoding
Copy link
Contributor Author

Closing this in favor of #633

@roziscoding roziscoding deleted the set-webhook-util branch August 8, 2023 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: SetWebhookUtil component
2 participants