-
Notifications
You must be signed in to change notification settings - Fork 946
Update README #5214
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
Update README #5214
Conversation
|
Changeset File Check ✅
|
Co-authored-by: Feiyang <[email protected]>
packages/firebase/README.md
Outdated
@@ -177,7 +169,9 @@ $ npm install --save firebase | |||
In your code, you can access Firebase using: | |||
|
|||
```js | |||
var firebase = require('firebase'); | |||
var firebase = require('firebase/app').default; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to double check that Node needs the .default too? I vaguely remember maybe not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, if you want to get typings. Can you also update var
s to const
s?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
Binary Size ReportAffected SDKs
Test Logs |
packages/firebase/README.md
Outdated
@@ -1,4 +1,4 @@ | |||
[](https://travis-ci.org/firebase/firebase-js-sdk) | |||
<!-- [](https://travis-ci.org/firebase/firebase-js-sdk) --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be deleted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that leaving the comment would be a good reminder to put a new badge there if we go through the README in the future and we have a new service by then. I guess it's better to explicitly put a TODO instead.
require('firebase/app').default
per our ESM module changes in 8.0.0