Skip to content

Conversation

@joostfarla
Copy link

This PR fixes a wrong import statement in the TypeScript example.

While using this method, I got a console warning:

It looks like you're using the development build of the Firebase JS SDK.
When deploying Firebase apps to production, it is advisable to only import
the individual SDK components you intend to use.

For the module builds, these are available in the following manner
(replace <PACKAGE> with the name of a component - i.e. auth, database, etc):

CommonJS Modules:
const firebase = require('firebase/app');
require('firebase/<PACKAGE>');

ES Modules:
import firebase from 'firebase/app';
import 'firebase/<PACKAGE>';

But it appears this is the only way to be able to work with the full typing definitions of the SDK.
Therefore I was wondering: Is there a way to suppress this?

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

jshcrowthe added a commit that referenced this pull request May 10, 2018
@jshcrowthe jshcrowthe mentioned this pull request May 10, 2018
jshcrowthe pushed a commit that referenced this pull request May 10, 2018
* Revert "SubImport Typings Fix (#796)"

This reverts commit 3110d6b.

* Fixes #799

* Fixes #791, #800

* [AUTOMATED]: Prettier Code Styling

* Add submodule typings tests
@jshcrowthe
Copy link
Contributor

Hey @joostfarla if you upgrade to the latest firebase (i.e. 5.0.2), you should be good to go! We addressed this bug a little differently but it is now fixed.

@jshcrowthe jshcrowthe closed this May 10, 2018
@joostfarla
Copy link
Author

Thanks @jshcrowthe!

After upgrading to v5.0.2, When I use the documented way of importing in TS:

import * as firebase from 'firebase/app';

I now get an error:

Uncaught TypeError: firebase.initializeApp is not a function

Can you give an example of how to use this correctly?

@jshcrowthe
Copy link
Contributor

That is a great question! It seems our documentation is wrong there, it should be:

import firebase from 'firebase/app';

If you'd like to submit a PR for that let's get it merged!

@joostfarla
Copy link
Author

When I try:

import firebase from 'firebase/app';

I'm getting this warning:

[ts] Module '"/.../node_modules/firebase/index"' has no default export.

@paulstelzer
Copy link
Contributor

paulstelzer commented May 14, 2018

@jshcrowthe I can confirm what @joostfarla saying - i also get "[ts] Module '"/.../node_modules/firebase/index"' has no default export." using [email protected] and [email protected] and [email protected]

The funny part is: Using [email protected] with [email protected] and ionic with [email protected] , it's working

@jshcrowthe
Copy link
Contributor

jshcrowthe commented May 14, 2018

@joostfarla or @paulstelzer can you post your tsconfig.json so I can look into this? I'm using the latest (as of writing this is 2.8.3) and I get no error...

Additionally, it may be better to move this to a separate issue, so if one of you would be willing to do that, that'd be great.

@paulstelzer
Copy link
Contributor

paulstelzer commented May 14, 2018

@jshcrowthe very strange - used the tsconfig.json from [email protected] (installed completely new). See github -> https://github.com/paulstelzer/angular6-firebase/ - I put it into ' /src/app/app.component.ts ' (Error: has no default export and so building is not possible)

{
  "compileOnSave": false,
  "compilerOptions": {
    "baseUrl": "./",
    "outDir": "./dist/out-tsc",
    "sourceMap": true,
    "declaration": false,
    "moduleResolution": "node",
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "target": "es5",
    "typeRoots": [
      "node_modules/@types"
    ],
    "lib": [
      "es2017",
      "dom"
    ]
  }
}

@paulstelzer
Copy link
Contributor

paulstelzer commented May 14, 2018

@jshcrowthe GOT IT :) Your assumption with tsconfig.json was right - [email protected] not using

"allowSyntheticDefaultImports": true

in their tsconfig.json. If you add this line to your tsconfig.json, everything works fine!

@jshcrowthe
Copy link
Contributor

Awesome 👍 I thought that might be the issue. Thanks for taking a second look @paulstelzer

@joostfarla
Copy link
Author

Hmm I don't think we should enforce the use of the allowSyntheticDefaultImports option. The user should be able to use both, with and without.

When allowSyntheticDefaultImports is enabled and do:

import firebase from 'firebase/app';

And when allowSyntheticDefaultImports is disabled and do:

import * as firebase from 'firebase/app';

...should behave exactly the same. The latter currently gives a TypeScript error:

[ts] Module '"/.../node_modules/firebase/index"' has no default export.

@jshcrowthe
Copy link
Contributor

jshcrowthe commented May 17, 2018

@joostfarla I appreciate what you're saying and the fix that will enable that will be included in the next release. That said the two things in ES Module world, are actually completely different.

Consider a module like the one below:

// Name: foobar

export function foo() {}
export default function bar() {}

The syntaxes you referenced above do two different things:

import thing from 'foobar';
thing() // References the function `bar` above;
import * as thing from 'foobar';
thing.foo() // `thing` references an object containing `foo` and a property called `default` that references `bar`

I'd recommend doing the import firebase from 'firebase/app' version for forward compatibility, however the fix that will enable this should go out soon!

@joostfarla
Copy link
Author

@jshcrowthe Allright, thanx!

@firebase firebase locked and limited conversation to collaborators Oct 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants