alttpo / sni

SNES Interface with gRPC API
MIT License
46 stars 12 forks source link

Update gRPCweb example #33

Closed kupppo closed 10 months ago

kupppo commented 10 months ago

The examples/grpcweb folder currently only works with Windows. This PR updates a couple of things to address this:

  1. gen.sh has been changed to be executable. This was done with chmod +x ./gen.sh.
  2. gen.sh is now runs cross-platform. This previously failed due to the path of protoc-gen-ts having a hardcoded .cmd at the end which does not exist for Linux/Mac users.
  3. The generate script can now be called via npm. In keeping with npm conventions, the build script is now defined in the package.json file. Running npm run build will do the same thing as ./gen.sh.

I've also added a README.md to help get anyone looking to get started.

JamesDunne commented 10 months ago

lgtm! I'll run it locally first and merge if it looks good. thank you.

kupppo commented 10 months ago

This has been tidied up a bit and removed old dependencies. I've confirmed it runs on Windows, M1 Mac and Intel Mac.

JamesDunne commented 10 months ago

I like the new cleanups but I think you went one step too far with removing src/index.ts.

The original intention of this grpcweb example was to be a minimal working example of how to use grpc-web to talk to SNI directly from a web browser, which isn't normally possible with vanilla grpc.

kupppo commented 10 months ago

No worries. I got caught up in cleaning up that I forgot that this is an example and not a package. I'll get a demo with this working on a basic web site and re-request review.

kupppo commented 10 months ago

@JamesDunne This is ready for review again. I've used Next.js to make an example page that uses DeviceList. There are instructions in the README for how to get the page to run. Here are some screenshots of the example page running.

Screenshot 2023-12-07 at 12 41 07 PM Screenshot 2023-12-07 at 12 41 46 PM Screenshot 2023-12-07 at 12 42 09 PM