Page MenuHomePhabricator

[keyserver] frog hello world example
AcceptedPublic

Authored by will on Fri, Nov 15, 12:06 PM.
Tags
None
Referenced Files
F3352149: D13947.id45851.diff
Sat, Nov 23, 4:48 AM
F3349481: D13947.id45912.diff
Fri, Nov 22, 6:06 PM
F3349475: D13947.id45890.diff
Fri, Nov 22, 6:06 PM
F3349466: D13947.id45869.diff
Fri, Nov 22, 6:05 PM
Unknown Object (File)
Thu, Nov 21, 8:39 PM
Unknown Object (File)
Thu, Nov 21, 11:58 AM
Unknown Object (File)
Thu, Nov 21, 7:32 AM
Unknown Object (File)
Wed, Nov 20, 2:47 PM
Subscribers

Details

Reviewers
ashoat
Summary

This diff implements a hello world Frog application and serves it on port 3001.

Depends on D13927

Test Plan

GET requests to the keyserver at localhost:3001 contain valid HTML document in the response.

Used ngrok to create a publicly accessible url forwarding requests to locahost:3001. Used this url to validate the frame with the warpcast frame validator at https://warpcast.com/~/developers/frames?url=https://81fb-69-201-171-11.ngrok-free.app

Diff Detail

Repository
rCOMM Comm
Branch
wyilio/frames
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

will added inline comments.
keyserver/src/frog/frog.js
13–22 ↗(On Diff #45850)

The JSX here will be replaced later in the stack. This is a placeholder

Harbormaster returned this revision to the author for changes because remote builds failed.Fri, Nov 15, 1:30 PM
Harbormaster failed remote builds in B32687: Diff 45850!
will requested review of this revision.Fri, Nov 15, 1:56 PM
keyserver/.eslintrc.json
13 ↗(On Diff #45850)

We talked about doing this via a // eslint-disable macro... why'd you decide against it?

ashoat requested changes to this revision.Fri, Nov 15, 2:03 PM

Back to you with question

This revision now requires changes to proceed.Fri, Nov 15, 2:03 PM
keyserver/.eslintrc.json
13 ↗(On Diff #45850)

Realizing I can do this with the macro specifically for the react/react-in-jsx-scope rule. I mistakenly thought disabling rules for entire files could only be done with eslintrc.json. A quick look through the codebase confirmed we should use the macro.

I found several examples of disabling certain blocks of code by surrounding it with:
/* eslint-disable react/react-in-jsx-scope */
/* eslint-enable react/react-in-jsx-scope */

We do seem to disable eslint rules for the entire file in a few places in native (ex. native/index.js) and scripts, but I think the block approach seems appropriate for this diff.

use /* eslint-disable approach

ashoat added inline comments.
keyserver/src/frog/frog.js
8 ↗(On Diff #45869)

Can we rename this to startFrogHonoServer?

12 ↗(On Diff #45869)

I'd probably just make this apply to the whole file, on the assumption that Frog never uses React and always wants its JSX treated as Hono JSX

This revision is now accepted and ready to land.Mon, Nov 18, 6:22 PM

feedback and changed button label from "Invite Link" to "Join chat"

will marked 2 inline comments as done.Tue, Nov 19, 9:12 PM
keyserver/flow-typed/npm/frog_v0.18.x.js
10

Are we sure that these are the only two properties in this object? If not, use ...

14–15

Are we sure that these are the only two props for this React component? If not, use ...

will marked 2 inline comments as done.Wed, Nov 20, 8:20 AM