-
Notifications
You must be signed in to change notification settings - Fork 926
Roll bundle with prebuilt #4128
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
Roll bundle with prebuilt #4128
Conversation
… as a separate module.
|
} | ||
] | ||
}), | ||
...util.es2017Plugins('browser', /* mangled= */ false) |
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.
This should be mangled.
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.
Done.
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.
As discussed offline, this should be false.
} | ||
], | ||
plugins: util.es2017ToEs5Plugins(), | ||
external: util.resolveNodeExterns, |
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.
This should be resolveBrowserExterns
.
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.
Done.
} | ||
] | ||
}), | ||
...util.es2017Plugins('rn', /* mangled= */ false) |
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.
This should be mangled.
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.
Done.
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.
Same as browser build, this should be false.
Co-authored-by: Sebastian Schmidt <[email protected]>
* Split up firestore and firestore-memory build We need to verify that the Node build still works * Fix sebstian's memory build my separating it further.. * To mangle or not to mangle. * Add the missing return! Co-authored-by: Sebastian Schmidt <[email protected]>
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.
Last round, but basically LGTM
@@ -26,15 +25,8 @@ import { SnapshotVersion } from './snapshot_version'; | |||
* Represents a Firestore bundle saved by the SDK in its local storage. | |||
*/ | |||
export interface Bundle { | |||
/** |
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.
Please add back.
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.
Done.
readonly id: string; | ||
|
||
/** Schema version of the bundle. */ |
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.
Please add back.
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.
Done.
readonly version: number; | ||
|
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.
Please add back.
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.
Done.
@@ -46,9 +38,7 @@ export interface Bundle { | |||
* Represents a Query saved by the SDK in its local storage. | |||
*/ | |||
export interface NamedQuery { | |||
/** The name of the query. */ |
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.
Please add back.
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.
Done.
readonly name: string; | ||
/** The underlying query associated with `name`. */ |
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.
Please add back.
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.
Done.
* Add bundles to d.ts and rearrange bundles source code for building it as a separate module (#4120) * Add bundles to d.ts and rearrange bundles source code for building it as a separate module. * Roll bundle with prebuilt (#4128) Build firestore sdks with prebuilt to support bundle as a prototype patched feature. * Add JSDoc for bundles (#4155) Add JSDoc for bundles. * Create changeset * Update old-lobsters-pull.md * A few fixes for Firestore bundle sdk builds (#4177) * Actually creates firebase/firestore/bundle package.json * Fix standard node/browser builds for bundles * Make it release from remote branch. * Add bundle to firebase complete build and fix missing proto for memory build. * Make rollup.config.js more organized. * Revert "Make it release from remote branch." This reverts commit 2c91fd1. * 2017 -> 2020 * Update comment. * Address comments * Update .changeset/old-lobsters-pull.md Co-authored-by: Sebastian Schmidt <[email protected]> * Update .changeset/old-lobsters-pull.md Co-authored-by: Sebastian Schmidt <[email protected]> * Add toc. * Remove webpack change. Co-authored-by: Sebastian Schmidt <[email protected]>
No description provided.