Firestore Standalone Build #293
Conversation
jshcrowthe
requested review from
mikelehen,
schmidt-sebastian and
wilhuff
as code owners
| * limitations under the License. | ||
| */ | ||
|
|
||
| // Local console build for |
There was a problem hiding this comment.
This comment seems incomplete. Could you explain how this file works (i.e. what it plugs into, etc)?
| * | ||
| * This will be kept up to date as this changes | ||
| */ | ||
| regex: /^_[^_][^lat]|[^_]_$/ |
There was a problem hiding this comment.
This regex doesn't do what you say. You probably broke the _lat property but are tests aren't sensitive enough to see it.
The first branches matches underscore followed by not underscore followed any single character that is none of "l", "a", or "t".
Better to treat _lat as a reserved keyword.
Alternatively, you can phrase this with a negative lookahead, something like:
/^(?!_lat)_[^_]|[^_]_$/
There was a problem hiding this comment.
Turns out they don't need the minification. So this will be pulled.
| configureForFirebase(instance); | ||
| } | ||
|
|
||
| registerFirestore(firebase); |
There was a problem hiding this comment.
This seems to be identical to index.ts except it expects firebase to be an existing global, but I don't think this is actually what console needs.
firebase will not exist in the console (or if it is, it isn't what we want) and we need to expose our public constructor to the console environment somewhere (see goog_module_config.ts for how we did this previously).
So unless I'm confused, this isn't going to work for console folks. If you haven't already, I'd work with the console folks to get an end-to-end solution working before sending this out before trying to get this checked in... and it would be great if you could document how the integration works somewhere (how to trigger this build, what it exposes, how console consumes it, etc.).
There was a problem hiding this comment.
Was up working with @samhorlbeck and was under the impression that FB existed and that this was all that was needed. I'll sync back up w/ him and @tstirrat.
There was a problem hiding this comment.
I could be confused. As long as we have a (manual) end-to-end "build from github => integrate into console => console still works" validation, then I'll be happy. :-)
|
Closing this temporarily. I am testing some things and will open when I have a better proposal |
This is a standalone build for firestore. This is for the Firestore console. It is only manually ran as we won't need to do really do this outside of console building.
cc: @samhorlbeck