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 a form helper to reduce boilerplate #40

Merged
merged 3 commits into from
May 20, 2021
Merged

add a form helper to reduce boilerplate #40

merged 3 commits into from
May 20, 2021

Conversation

scytacki
Copy link
Member

@scytacki scytacki commented May 20, 2021

hopefully this doesn't make things confusing.
the goal is to help a developer focus on the logic of using the token service

this also adds a npm run start:secure script.

hopefully this doesn't make things confusing.
the goal is to help a developer focus on the logic of using the token service

this also adds a secure:start option
@scytacki scytacki requested a review from pjanik May 20, 2021 04:32
@scytacki scytacki marked this pull request as ready for review May 20, 2021 04:32
Copy link
Member

@pjanik pjanik left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍
I've left one feedback re "TODO hacky" comment.

const [filePublicUrl, setFilePublicUrl] = useState("");

// TODO hacky
const tokenServiceEnv: "dev" | "staging" = rawTokenServiceEnv as "dev" | "staging";
Copy link
Member

Choose a reason for hiding this comment

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

It seems pretty okay to me in practice. If we wanted to be super safe, we could possibly add something like that:

Suggested change
const tokenServiceEnv: "dev" | "staging" = rawTokenServiceEnv as "dev" | "staging";
const tokenServiceEnv: "dev" | "staging" = rawTokenServiceEnv === "dev" || rawTokenServiceEnv === "staging" ? rawTokenService : "staging";

or even some better logic there. This should prevent updates of tokenServiceEnv while the user is typing and assigning it to values like "d", "de", "st", "sta", "stagi", etc. But I don't think it matters at all in practice in this case.

@scytacki scytacki merged commit d62e5e6 into master May 20, 2021
@scytacki scytacki deleted the use-form-helper branch May 20, 2021 16:09
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.

None yet

2 participants