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: grammy/web builds for all plugins and storage adapters #372

Closed
dandaka opened this issue Feb 27, 2023 · 15 comments
Closed

feat: grammy/web builds for all plugins and storage adapters #372

dandaka opened this issue Feb 27, 2023 · 15 comments

Comments

@dandaka
Copy link

dandaka commented Feb 27, 2023

Now

All Storage Adapters (and Session as it is dependent on them) do not support working without Node API. So Grammy can't be used with the sessions plugin on Cloudflare Workers (and similar Edge platforms).

Expected

Some/all Storage Adapters can be imported without Node API dependency.

@KnorpelSenf
Copy link
Member

This is generally the case with all plugins. The core repository is the only project that has a web build. We need to add a similar setup to all ~30 repos. Contributions are very welcome!

@KnorpelSenf KnorpelSenf changed the title Session / Storage Adapters support for grammy/web build feat: grammy/web builds for all plugins and storage adapters Feb 28, 2023
@dandaka
Copy link
Author

dandaka commented Feb 28, 2023

@KnorpelSenf did you have any branch, where you implemented the feature for the core rep?

@KnorpelSenf
Copy link
Member

KnorpelSenf commented Feb 28, 2023

grammY is natively written for Deno. Deno code is compatible with the browser and cloudflare. That's why a simple deno bundle pass to remove the type annotations already gave us a web build that could run in the browser and in cloudflare.

The only problem used to be that cloudflare didn't support top-level await. (This was added by now.) This is why we sort of always had web builds, so it wasn't really an isolated feature we added at some point.

The tooling to create these JavaScript bundles has evolved over time. I think the most useful PQ to check is #339 which takes the existing web bundle and adds it to the npm package in order for it to be consumed by wrangler.

@KnorpelSenf
Copy link
Member

@dandaka are you interested in working on this? I'd be happy to review and assist. I'm sure @roziscoding will also be able to answer any questions :)

@dandaka
Copy link
Author

dandaka commented Feb 28, 2023

I am a junior dev at best, learning React + Typescript + Node + modern JS at the same time while developing a pet project. So while I am interested, it might take a gigantic effort at my current level :)

@KnorpelSenf
Copy link
Member

Ah I'm not worried about that. grammY is the right place to achieve great things no matter what your level of expertise is. There are many stories to tell of novice coders who grew far beyond themselves here, so if you'd like to give it a try, I'm fairly certain that you'll succeed :)

@PonomareVlad
Copy link
Member

I encountered a similar problem when deploying on Vercel Edge Runtime.

If importing the grammY core for web is not a problem:

import { Bot } from "grammy/web"; // instead of "grammy"

The plugins will still use the default import:

import { /*...*/ } from "grammy";

To solve this problem, I tried two methods:

  1. Using an override package that re-exports the default grammy export from the grammy/web path.
  2. Using custom build of grammY package with default web export.

The first option seems to be the most compatible, but it have an issue with TypeScript types import.

At the moment, I have not found a way to fix this, and I do not have lot experience with TypeScript.

As a result, for my projects, I decided to use the second method, in which the types imports correctly and all plugins that do not depend on built-in Node.js modules too.

Working example 🙂

@dandaka
Copy link
Author

dandaka commented Apr 29, 2023

I have moved to Deno + Deno Deploy and never looked back!

@KnorpelSenf
Copy link
Member

Well that's certainly the smart choice, but people will still need /web exports of all packages, so this issue is still relevant

@PonomareVlad
Copy link
Member

@KnorpelSenf Can you explain or give instructions on how it would be correct to implement web exports in plugins ?

@roziscoding
Copy link
Contributor

I believe it'd be something similar to what's done for the core package

@KnorpelSenf KnorpelSenf moved this to To do in Coding Jun 1, 2023
@KnorpelSenf
Copy link
Member

KnorpelSenf commented Jun 16, 2023

Ideally, the solution looks like this:

  1. create a new repo with shared build scripts for all other repos
  2. migrate to dnt
  3. take over and adjust the bundling script from this repo
  4. use the shared build script in deno tasks in all repos

I am not sure when to tackle this, but I find it hard to imagine that this is possible without step 2 because d2n requires a config file, and this config file is different per project. Unless we find an easy way to solve this (long CLI flags?), we need to move to a tool that requires no config.

/cc @wojpawlik

@wojpawlik
Copy link
Contributor

Overkill. Other packages already can deno run https://raw.githubusercontent.com/grammyjs/grammY/main/bundling/bundle-web.ts.

The plugins will still use the default import:

Good point @PonomareVlad. Entry point could provide "node" and "default" consitional exports. This would obsolete "grammyjs/web".

@KnorpelSenf
Copy link
Member

This would obsolete "grammyjs/web".

I like that a lot. Can I ask you to procrastinate a little more and open a PQ?

@KnorpelSenf
Copy link
Member

Closing in favour of #559

@github-project-automation github-project-automation bot moved this from To do to Done in Coding May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

No branches or pull requests

5 participants