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

Initialize firebase and added firebase.js config file #10

Merged
merged 7 commits into from
Mar 20, 2023

Conversation

ranegray
Copy link
Collaborator

This should be everything we need to start coding. Everything else can be handled in the firebase console which I will add people too.

@ranegray
Copy link
Collaborator Author

This is my first ever PR so lemme know if I'm dumb or did something wrong.

@ranegray ranegray linked an issue Mar 16, 2023 that may be closed by this pull request
firebaseConfig.js Outdated Show resolved Hide resolved
firebaseConfig.js Outdated Show resolved Hide resolved
firebaseConfig.js Outdated Show resolved Hide resolved
@MacAndersonUche
Copy link
Collaborator

Nice Looks Good to me

@MacAndersonUche
Copy link
Collaborator

@toreylittlefield please review this

@toreylittlefield
Copy link
Collaborator

@MacAndersonUche / @ranegray you want to fix this up so we can merge it?

@ranegray
Copy link
Collaborator Author

@toreylittlefield do i just delete the conflict markers using the web editor?

@toreylittlefield
Copy link
Collaborator

@ranegray first switch to main.

Then pull down the changes git pull

Then switch back to your branch here.

Then merge main onto your branch git merge origin/main

You'll have the chance to solve the merge conflicts. I recommend just solving the conflicts in the package.json. Looks like new files have been installed. Don't worry about the package-lock it'll be too big a pain in the ass.

once you resolve the merge conflicts in package.json. Just delete package-lock and then run npm install it'll create a new package lock. Push up all the new changes for the package.json and package-lock 👍

Comment on lines 10 to 17
const firebaseConfig = {
apiKey: process.env.apiKey,
authDomain: process.env.authDomain,
projectId: process.env.projectId,
storageBucket: process.env.storageBucket,
messagingSenderId: process.env.messagingSenderId,
appId: process.env.appId,
measurementId: process.env.measurementId
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe these keys need to be prefix with NEXT_PUBLIC to work clientside. I don't know enough about firebase yet but are these configuration values supposed to be exposed client side or server side only?

I think only firebase admin keys should be private and secured server side?

Also nextjs will work out of the box with a .local.env file or whatever but not .env see the docs.

https://nextjs.org/docs/basic-features/environment-variables#exposing-environment-variables-to-the-browser

Can you also just confirm somehow that the configuration is working?

@MacAndersonUche @ranegray

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

From the docs (https://firebase.google.com/docs/projects/api-keys):

Unlike how API keys are typically used, API keys for Firebase services are not used to control access to backend resources; that can only be done with Firebase Security Rules (to control which users can access resources) and App Check (to control which apps can access resources).

Usually, you need to fastidiously guard API keys (for example, by using a vault service or setting the keys as environment variables); however, API keys for Firebase services are ok to include in code or checked-in config files.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ranegray you're right but maybe safer for now to use the .env.local because we haven't set the Firebase Security Rules

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is a .env.local i created it. do you want me to put that file somewhere. obviously its not going to push to GitHub

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ranegray yep you're right no need to share it 😄 Just please update the variable name for those secrets to use the NEXT_PUBLIC syntax thank you!

@toreylittlefield
Copy link
Collaborator

toreylittlefield commented Mar 20, 2023

@ranegray nice work and thanks! I got it working on my end using my own firebase credentials 💯 .

I'll merge for now but my suggestion is to move out this file into the lib directory under a firebase dir. So lib/firebase/firebaseConfig.js or something.

Another thing is the env variable names should be all uppercase and separated with an underscore. This is just JS convention. It doesn't change anything.

NEXT_PUBLIC_apiKey -> NEXT_PUBLIC_API_KEY

Last thing
const firebaseAnalytics = getAnalytics(app); throws an a window not defined error. It's a nextjs thing because of SSR. I think we don't need the analytics right now so we can remove it and figure out the solution later.

I'll create a separate ticket for these issues here #20.

Again great job and congrats!

@ranegray ranegray deleted the rg/issue-6-setup-firebase-account branch March 21, 2023 22:42
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.

Setup Firebase account
3 participants