Skip to content

Memory-only Firestore build #2608

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

Merged
merged 23 commits into from
Mar 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions integration/firestore/gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ function clean() {
return del(['temp/**/*', 'dist/**/*']);
}

function isPersistenceEnabled() {
return process.env.INCLUDE_FIRESTORE_PERSISTENCE !== 'false';
}

function copyTests() {
/**
* NOTE: We intentionally don't copy src/ files (to make sure we are only
Expand All @@ -37,7 +41,9 @@ function copyTests() {
const firebaseAppSdk = 'firebase/app/dist/index.esm.js';
const firebaseFirestoreSdk = resolve(
__dirname,
'../../packages/firestore/dist/index.esm.js'
isPersistenceEnabled()
? '../../packages/firestore/dist/index.esm.js'
: '../../packages/firestore/dist/index.memory.esm.js'
);
return gulp
.src(
Expand All @@ -63,7 +69,14 @@ function copyTests() {
*/
/import\s+firebase\s+from\s+('|")[^\1]+firebase_export\1;?/,
`import firebase from '${firebaseAppSdk}';
import '${firebaseFirestoreSdk}';`
import '${firebaseFirestoreSdk}';

if (typeof process === 'undefined') {
process = { env: { INCLUDE_FIRESTORE_PERSISTENCE: '${isPersistenceEnabled()}' } } as any;
} else {
process.env.INCLUDE_FIRESTORE_PERSISTENCE = '${isPersistenceEnabled()}';
}
`
)
)
.pipe(
Expand Down
15 changes: 8 additions & 7 deletions integration/firestore/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
"version": "1.0.1",
"private": true,
"scripts": {
"build": "gulp compile-tests",
"build:deps": "cd ../../ ; yarn build",
"pretest:manual": "yarn build",
"pretest:debug": "yarn build:deps && yarn build",
"test": "echo 'Automated tests temporarily disabled, run `yarn test:manual` to execute these tests'",
"test:manual": "karma start --single-run",
"test:debug": "karma start --auto-watch --browsers Chrome"
"build:deps": "cd ../../packages/firestore ; yarn build",
"build:persistence": "INCLUDE_FIRESTORE_PERSISTENCE=true gulp compile-tests",
"build:memory": "INCLUDE_FIRESTORE_PERSISTENCE=false gulp compile-tests",
"test": "yarn build:memory; karma start --single-run; yarn build:persistence; karma start --single-run;",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much like yarn test does everything, I think it's reasonable to still have a yarn build that builds everything.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, gulp uses the same output folder for both modes. This means that the builds overwrite each other. We can probably use different output folders, but this would require some extra plumbing. Let me know if this is something that you want me to do.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. I see. It's certainly not something to tackle in this PR. If gulp were better about doing incremental builds this could be worthwhile, but I suspect it's not going to actually buy us much or possibly even anything.

"test:persistence": " yarn build:persistence; karma start --single-run",
"test:memory": "yarn build:memory; karma start --single-run",
"test:debug:persistence": "yarn build:deps; yarn build:persistence; karma start --auto-watch --browsers Chrome",
"test:debug:memory": "yarn build:deps; yarn build:memory; karma start --single-run"
},
"devDependencies": {
"@types/mocha": "7.0.2",
Expand Down
23 changes: 23 additions & 0 deletions packages/firebase/firestore/memory/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/**
* @license
* Copyright 2020 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* This file serves as the public entrypoint for users that import
* `firebase/firestore/memory`.
*/

import '../../../firestore/dist/index.memory.esm';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's still the case that looking at this file I can't tell why it exists, what bits of the build it interacts with, and whether or not I should use for anything when e.g. writing Firestore tests. I think this file is establishing the public interface to Firestore as seen when a user imports @firebase/firestore/memory, right? Comments that address these concerns would be helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a short comment that explains this a little bit.

7 changes: 7 additions & 0 deletions packages/firebase/firestore/memory/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "firebase/firestore/memory",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this also have a description and a license and all those standard fields?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is modeled after the package.json files in firebase@next, but I believe you are right. @Feiyang1 should we add license/description fields?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no license needed for json files.
Those fields are not needed in this file because they are already in the root package.json. You can provide a description for sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Descriptions added to this package.json and top-level package.json.

"description": "A memory-only build of the Cloud Firestore JS SDK.",
"main": "dist/index.cjs.js",
"module": "dist/index.esm.js",
"typings": "../../empty-import.d.ts",
}
1 change: 1 addition & 0 deletions packages/firebase/firestore/package.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"name": "firebase/firestore",
"description": "The Cloud Firestore component of the Firebase JS SDK.",
"main": "dist/index.cjs.js",
"module": "dist/index.esm.js",
"typings": "../empty-import.d.ts"
Expand Down
33 changes: 32 additions & 1 deletion packages/firebase/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,32 @@ const componentBuilds = pkg.components
})
.reduce((a, b) => a.concat(b), []);

const firestoreMemoryBuilds = [
{
input: `firestore/memory/index.ts`,
output: [
{
file: resolve('firestore/memory', pkg.main),
format: 'cjs',
sourcemap: true
},
{
file: resolve('firestore/memory', pkg.module),
format: 'es',
sourcemap: true
}
],
plugins,
external
},
{
input: `firestore/memory/index.ts`,
output: createUmdOutputConfig(`firebase-firestore.memory.js`),
plugins: [...plugins, uglify()],
external: ['@firebase/app']
}
];

/**
* Complete Package Builds
*/
Expand Down Expand Up @@ -260,4 +286,9 @@ const completeBuilds = [
}
];

export default [...appBuilds, ...componentBuilds, ...completeBuilds];
export default [
...appBuilds,
...componentBuilds,
...firestoreMemoryBuilds,
...completeBuilds
];
41 changes: 41 additions & 0 deletions packages/firestore/index.memory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* @license
* Copyright 2020 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import firebase from '@firebase/app';
import { FirebaseNamespace } from '@firebase/app-types';

import { Firestore } from './src/api/database';
import { MemoryPersistenceProvider } from './src/local/memory_persistence';
import { configureForFirebase } from './src/platform/config';

import './register-module';
import './src/platform_browser/browser_init';

import { name, version } from './package.json';

/**
* Registers the memory-only Firestore build with the components framework.
*/
export function registerFirestore(instance: FirebaseNamespace): void {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done (here and elsewhere)

configureForFirebase(
instance,
(app, auth) => new Firestore(app, auth, new MemoryPersistenceProvider())
);
instance.registerVersion(name, version);
}

registerFirestore(firebase);
41 changes: 41 additions & 0 deletions packages/firestore/index.node.memory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* @license
* Copyright 2020 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import firebase from '@firebase/app';
import { FirebaseNamespace } from '@firebase/app-types';

import { Firestore } from './src/api/database';
import { MemoryPersistenceProvider } from './src/local/memory_persistence';
import { configureForFirebase } from './src/platform/config';
import './register-module';
import './src/platform_node/node_init';

import { name, version } from './package.json';

/**
* Registers the memory-only Firestore build for Node with the components
* framework.
*/
export function registerFirestore(instance: FirebaseNamespace): void {
configureForFirebase(
instance,
(app, auth) => new Firestore(app, auth, new MemoryPersistenceProvider())
);
instance.registerVersion(name, version);
}

registerFirestore(firebase);
15 changes: 13 additions & 2 deletions packages/firestore/index.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,26 @@
* limitations under the License.
*/
import firebase from '@firebase/app';
import { FirebaseNamespace } from '@firebase/app-types';

import { Firestore } from './src/api/database';
import { IndexedDbPersistenceProvider } from './src/local/indexeddb_persistence';
import { configureForFirebase } from './src/platform/config';

import './register-module';
import './src/platform_node/node_init';
import { FirebaseNamespace } from '@firebase/app-types';

import { name, version } from './package.json';

/**
* Registers the main Firestore Node build with the components framework.
* Persistence can be enabled via `firebase.firestore().enablePersistence()`.
*/
export function registerFirestore(instance: FirebaseNamespace): void {
configureForFirebase(instance);
configureForFirebase(
instance,
(app, auth) => new Firestore(app, auth, new IndexedDbPersistenceProvider())
);
instance.registerVersion(name, version, 'node');
}

Expand Down
19 changes: 14 additions & 5 deletions packages/firestore/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,25 @@
*/

import firebase from '@firebase/app';
import { configureForFirebase } from './src/platform/config';
import './register-module';
import './src/platform_browser/browser_init';

import { FirebaseNamespace } from '@firebase/app-types';

import { Firestore } from './src/api/database';
import { IndexedDbPersistenceProvider } from './src/local/indexeddb_persistence';
import { configureForFirebase } from './src/platform/config';
import { name, version } from './package.json';

import './register-module';
import './src/platform_browser/browser_init';

/**
* Registers the main Firestore build with the components framework.
* Persistence can be enabled via `firebase.firestore().enablePersistence()`.
*/
export function registerFirestore(instance: FirebaseNamespace): void {
configureForFirebase(instance);
configureForFirebase(
instance,
(app, auth) => new Firestore(app, auth, new IndexedDbPersistenceProvider())
);
instance.registerVersion(name, version);
}

Expand Down
9 changes: 9 additions & 0 deletions packages/firestore/memory/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "@firebase/firestore/memory",
"description": "A memory-only build of the Cloud Firestore JS SDK.",
"main": "../dist/index.memory.node.cjs.js",
"browser": "../dist/index.memory.cjs.js",
"module": "../dist/index.memory.esm.js",
"esm2017": "../dist/index.memory.esm2017.js",
"typings": "../dist/index.d.ts"
}
11 changes: 6 additions & 5 deletions packages/firestore/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@firebase/firestore",
"version": "1.12.1",
"description": "",
"version": "1.11.2",
"description": "The Cloud Firestore component of the Firebase JS SDK.",
"author": "Firebase <[email protected]> (https://firebase.google.com/)",
"scripts": {
"prebuild": "tsc -d --downlevelIteration --declarationDir dist/lib --emitDeclarationOnly && tsc -m es2015 --moduleResolution node scripts/*.ts ",
Expand All @@ -10,7 +10,7 @@
"dev": "rollup -c -w",
"lint": "eslint -c .eslintrc.js '**/*.ts' --ignore-path '../../.gitignore'",
"lint:fix": "eslint --fix -c .eslintrc.js '**/*.ts' --ignore-path '../../.gitignore'",
"prettier": "prettier --write 'src/**/*.js' 'test/**/*.js' 'src/**/*.ts' 'test/**/*.ts'",
"prettier": "prettier --write '*.ts' '*.js' 'src/**/*.js' 'test/**/*.js' 'src/**/*.ts' 'test/**/*.ts'",
"test": "run-s lint test:all",
"test:all": "run-p test:browser test:travis test:minified",
"test:browser": "karma start --single-run",
Expand All @@ -20,7 +20,7 @@
"test:node:persistence": "FIRESTORE_EMULATOR_PORT=8080 FIRESTORE_EMULATOR_PROJECT_ID=test-emulator USE_MOCK_PERSISTENCE=YES TS_NODE_CACHE=NO TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' nyc --reporter lcovonly -- mocha 'test/{,!(browser)/**/}*.test.ts' --require ts-node/register --require index.node.ts --require test/util/node_persistence.ts --opts ../../config/mocha.node.opts",
"test:node:persistence:prod": "USE_MOCK_PERSISTENCE=YES TS_NODE_CACHE=NO TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' nyc --reporter lcovonly -- mocha 'test/{,!(browser)/**/}*.test.ts' --require ts-node/register --require index.node.ts --require test/util/node_persistence.ts --opts ../../config/mocha.node.opts",
"test:travis": "ts-node --compiler-options='{\"module\":\"commonjs\"}' ../../scripts/emulator-testing/firestore-test-runner.ts",
"test:minified": "(cd ../../integration/firestore ; yarn test:manual)",
"test:minified": "(cd ../../integration/firestore ; yarn test)",
"prepare": "yarn build"
},
"main": "dist/index.node.cjs.js",
Expand All @@ -29,7 +29,8 @@
"esm2017": "dist/index.esm2017.js",
"license": "Apache-2.0",
"files": [
"dist"
"dist",
"memory/package.json"
],
"dependencies": {
"@firebase/component": "0.1.7",
Expand Down
Loading