search

firebase / firebase-js-sdk

Firebase Javascript SDK
https://firebase.google.com/docs/web/setup
Other
4.78k stars 876 forks source link

Fr: Update externs for Closure users #4592

Open GlennKnight opened 3 years ago

GlennKnight commented 3 years ago

[REQUIRED] Describe your environment

[REQUIRED] Describe the problem

The externs available at firebase-js-sdk/packages/firebase/externs/ are currently incomplete and incorrect which causes issues when using the Closure Compiler.

Products that are missing externs:

The externs for Firestore are missing definitions for the withConverter() method.

I think the get() method for firebase.firestore.Query should return a Promise to reflect the extern's comments and the documentation for this method:

/**
 * Executes the query and returns the results as a `QuerySnapshot`.
 *
 * @param {!firebase.firestore.GetOptions=} options An options object to
 *   configure how the data is retrieved.
 *
 * @return {!firebase.firestore.QuerySnapshot}
 *   A promise that will be resolved with the results of the query.
 */
firebase.firestore.Query.prototype.get = function (options) {};

would become:

/**
 * Executes the query and returns the results as a `QuerySnapshot`.
 *
 * @param {!firebase.firestore.GetOptions=} options An options object to
 *   configure how the data is retrieved.
 *
 * @return {!Promise<!firebase.firestore.QuerySnapshot>}
 *   A promise that will be resolved with the results of the query.
 */
firebase.firestore.Query.prototype.get = function (options) {};

The externs also produce several warnings when using --jscomp_warning=lintChecks due to use of var over let or const.

google-oss-bot commented 3 years ago

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

Feiyang1 commented 3 years ago

Hi, thanks for the report! We kinda stopped updating the externs because we thought few people use them. Would you mind sharing how you consume the SDK?

Also it's a manual process to update the externs today, which is tedious and error prone, and as a result they tend to fall out of sync eventually. I wonder if the best way forward for closure support is to have consumers generate externs from typings automatically using tools like https://github.com/angular/tsickle, instead of us bundling them in the package.

GlennKnight commented 3 years ago

Thanks for getting back to me.

Ahhh what a shame, I had suspected that I was the only person still using them but saw a glimmer of hope when I saw the firebase-auth externs where updated a couple of months ago.

My set-up is something like the following:

<body> 
  <script src="/__/firebase/8.2.10/firebase-app.js"></script>
  <script src="/__/firebase/8.2.10/firebase-auth.js"></script>
  <script src="/__/firebase/8.2.10/firebase-firestore.js"></script>
  <script src="/__/firebase/init.js"></script>
  <script src="app.js"></script>
</body>

Where app.js is a vanilla JavaScript programme that has been run through the Closure Compiler with ADVANCED_OPTIMIZATIONS. I also include the firebase-externs in my src directory so that the compiler does not rename firebase specific functions and variables.

hsubox76 commented 1 year ago

Converting this to a feature request to gauge how much interest there is in adding back externs.