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

add isomorphic Deno-service worker example #488

Open
wants to merge 23 commits into
base: develop
Choose a base branch
from

Conversation

nickchomey
Copy link

@nickchomey nickchomey commented Jan 15, 2025

This adds an example for the typescript sdk that has isomorphic routing and rendering for deno and a service worker. If you go offline, everything will be served from the SW.

  • Uses Hono for the routing etc...
  • Deno tasks watch for changes and use esbuild to rebuild the service worker as-needed. And optionally minify it.

There's a few files that are probably extraneous - caddyfile, mise.toml. Not sure what to do about those since I'd like to have them in my own repo

@nickchomey
Copy link
Author

@Superpat care to give it a try? Im open to any feedback

@nickchomey nickchomey marked this pull request as draft January 15, 2025 23:00
@nickchomey
Copy link
Author

nickchomey commented Jan 15, 2025

Changed to draft. Just tinkering with a merge-signals mechanism to toggle an offline indicator (rather than a kludgy, non-declarative, non-signals mechanism that I was using)

@nickchomey nickchomey marked this pull request as ready for review January 16, 2025 00:44
@nickchomey nickchomey marked this pull request as draft January 16, 2025 14:16
@nickchomey
Copy link
Author

nickchomey commented Jan 16, 2025

Put in draft again - going to tinker some more, both for my own practice/learning, as well as to give people a more comprehensive TS example to get started with.

@Superpat
Copy link
Contributor

Great, I'll review / test when it's ready for merging !

@nickchomey nickchomey marked this pull request as ready for review January 16, 2025 19:08
@bencroker bencroker marked this pull request as draft January 17, 2025 14:41
@nickchomey
Copy link
Author

im going to keep tinkering here, but apart from whatever feedback you folks provide, I think it really is "done" for the purposes of the example

@nickchomey nickchomey marked this pull request as ready for review January 22, 2025 00:43
@bencroker
Copy link
Collaborator

Thanks for the PR! I’d like @Superpat to review this first, since he submitted the Typescript SDK.

@Superpat
Copy link
Contributor

I think I'll have time to review this on friday.

@Superpat
Copy link
Contributor

@nickchomey I'd say my main concern right now is that I want the examples to remain simple / easy to read, full application level examples should probably live in their own repo's. I'll let you know how that goes when I try running it, but we may need to slim it down a bit.

@nickchomey
Copy link
Author

nickchomey commented Jan 22, 2025

Very fair. I kind of just kept iterating out of curiosity.

Yet, it's actually pretty simple - 1 endpoint for the initial page, 1 for the sse content responses. They're in the shared-router file, which then gets imported by the deno and service-worker modules. Those just do some minimal stuff that's needed for each environment - that's the real value of this example. The rest was just reusing the stuff I did previously to test compressing sse responses.

The most complex stuff was related to displaying a message when online/offline - it started simple (event listener for those events) but got more complicated when I found that that didn't work when the page was refreshed. It still doesn't work perfectly though.

No one really needs to do anything other than have deno installed and run deno run dev (or whichever other task you want)

I'm perfectly happy to remove or change anything if you think it would make for a simpler, better example. Perhaps removing minification from the build file and deno tasks? Likewise just making the offline indicator only use data-on-offline and data-on-online?

I'll remove the caddyfile and mise.toml as it's ultimately not needed here, and I already scratched that itch re: compressing sse responses and documented it elsewhere. (also, someone just submitted a 2-line PR to Deno for an issue I created to add support for compressing text/event-stream responses. Hopefully it'll be merged soon).

I could also just completely replace the actual examples with one or two of the simple official examples, since all this really needs to do is show that d* can work in a service worker.

Let me know what you guys would like!

@nickchomey
Copy link
Author

nickchomey commented Jan 22, 2025

One thought - I could, in theory, just prebuild the serviceworker.js and remove the build script. Likewise simplify the deno tasks to just one option.

Though I thought that maybe including stuff like that would make it easier for people to modify and experiment with it.

An alternative could be adding a Readme (should be done anyway) that links to a separate repo where they could get a buildable example. That seems convoluted to me though.

Anyway, again, I'm perfectly happy to make any changes

@nickchomey nickchomey marked this pull request as draft January 22, 2025 15:14
@nickchomey
Copy link
Author

Ive put it back in draft - suddenly it isn't working from the service worker for me... Perhaps something in one of the dependencies changed...

@nickchomey
Copy link
Author

Im just dumb - evidently I didnt properly test the most recent changes that I had made (adding minification of template literals) and it was removing /n which apparently messes with D* typescript sdk.

@Superpat I know we had talked about minification of html when we were discussing compression. But given that compression is so effective and easy, and that minification of template literals really just amounts to removing line breaks and, perhaps, some comments which dont contribute much to filesize, I don't think its at all worth fixing the sdk to support doing this. Just compress js files and sse responses.

As mentioned, Deno will hopefully compress event-steam soon. Here's the PR for it denoland/deno#27776

And its dead-simple with caddy and other reverse proxies.

@nickchomey nickchomey marked this pull request as ready for review January 22, 2025 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants