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

fix(examples/firebase-auth-firestore): Fix imports and throw meaningful errors when missing .env #3352

Closed
wants to merge 2 commits into from

Conversation

penx
Copy link
Contributor

@penx penx commented Jun 1, 2022

Apply fixes as per comment by @mocon at #1811 (comment)

Also:

  • throw meaningul errors when environment variables are missing or can't be parsed.
  • check for existence of environment variable rather than cast to string

Closes: issue reported in comment #1811 (comment)

  • Docs N/A
  • Tests N/A

Testing Strategy:

cd examples/firebase-auth-firestore
yarn
yarn dev
# open http://localhost:3000
# assert "Error: Missing CLIENT_CONFIG environment variable" thrown as env variables are missing
# exit ^C
cp .env.example .env
yarn dev
# open http://localhost:3000
# assert "Error: Failed to parse private key" thrown as example env does not contain valid keys
# exit ^C
# edit .env and set CLIENT_CONFIG to invalid JSON
yarn dev
# open http://localhost:3000
# assert "Error: Invalid CLIENT_CONFIG environment variable"
# exit ^C
# edit .env and set valid keys
yarn dev
# open http://localhost:3000
# assert login page loads

I plan to follow up with a couple of other PRs:

Apply fixes as per comment by @mocon at remix-run#1811 (comment)

Also:

- throw meaningul errors when environment variables are missing or can't be parsed.
- check for existence of environment variable rather than cast to string
@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Jun 1, 2022

Hi @penx,

Welcome, and thank you for contributing to Remix!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at hello@remix.run.

Thanks!

- The Remix team

@penx penx changed the title Fix Firebase example fix(examples/firebase-auth-firestore): Fix imports and throw meaningful errors Jun 1, 2022
@penx penx changed the title fix(examples/firebase-auth-firestore): Fix imports and throw meaningful errors fix(examples/firebase-auth-firestore): Fix imports and throw meaningful errors on missing env Jun 1, 2022
@penx penx changed the title fix(examples/firebase-auth-firestore): Fix imports and throw meaningful errors on missing env fix(examples/firebase-auth-firestore): Fix imports and throw meaningful errors when missing .env Jun 1, 2022
@penx
Copy link
Contributor Author

penx commented Jun 1, 2022

by the way, for examples, it's safe to target the main branch, so you could close this PR and reopen the previous one

closing and opening a third PR as I've rebased so initial PR can't be reopened.

#3353

@penx penx closed this Jun 1, 2022
@MichaelDeBoey MichaelDeBoey added the duplicate This issue or pull request already exists label Jun 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed duplicate This issue or pull request already exists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants