Skip to content
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

Split database and database-compat #5276

Merged
merged 17 commits into from
Aug 20, 2021
Merged

Split database and database-compat #5276

merged 17 commits into from
Aug 20, 2021

Conversation

Feiyang1
Copy link
Member

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Aug 11, 2021

⚠️ No Changeset found

Latest commit: 267e9d9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

💥 An error occurred when fetching the changed packages and changesets in this PR
Some errors occurred when validating the changesets config:
The package "@firebase/database-compat" depends on the ignored package "@firebase/app-compat", but "@firebase/database-compat" is not being ignored. Please add "@firebase/database-compat" to the `ignore` option.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 11, 2021

Changeset File Check ⚠️

  • Changeset formatting error in following file:
    Package "firebase-compat-typings-test" must depend on the current version of "firebase-exp": "9.0.0-beta.8" vs "file:../../packages-exp/firebase-exp"
    ValidationError: Some errors occurred when validating the changesets config:
    The package "@firebase/database-compat" depends on the ignored package "@firebase/app-compat", but "@firebase/database-compat" is not being ignored. Please add "@firebase/database-compat" to the `ignore` option.
        at async run$1 (/home/runner/work/firebase-js-sdk/firebase-js-sdk/node_modules/@changesets/cli/dist/cli.cjs.dev.js:1096:16) {
      _error: Error
          at new ExtendableError (/home/runner/work/firebase-js-sdk/firebase-js-sdk/node_modules/extendable-error/bld/index.js:23:24)
          at new ValidationError (/home/runner/work/firebase-js-sdk/firebase-js-sdk/node_modules/@changesets/errors/dist/errors.cjs.dev.js:16:1)
          at parse (/home/runner/work/firebase-js-sdk/firebase-js-sdk/node_modules/@changesets/config/dist/config.cjs.dev.js:206:11)
          at Object.read (/home/runner/work/firebase-js-sdk/firebase-js-sdk/node_modules/@changesets/config/dist/config.cjs.dev.js:87:10)
          at async run$1 (/home/runner/work/firebase-js-sdk/firebase-js-sdk/node_modules/@changesets/cli/dist/cli.cjs.dev.js:1096:16)
    }
    

@Feiyang1
Copy link
Member Author

Moved some tests to @firebase/database-compat, because they use the compat API for testing. Most of them should be probably refactored and moved back to @firebase/database because they are really tests for the underlying functionalities, like transactions. And tests in @firebase/database-compat should only tests for the compat layer that it delegates correctly to the modular API.

It seems to be a non-trivial amount of work, and I don't think it's urgent right now, so maybe we can do it after GA? wdyt
@schmidt-sebastian

@schmidt-sebastian
Copy link
Contributor

The idea behind testing the compat layer was that it allowed us to not rewrite the tests, while ensuring that both the compat and the -exp layer get tested thoroughly. It gets a bit awkward now that the packages are split, so it is worth reevaluating.

For now, we can leave it as is as long as the database-compat tests are triggered for every database change.

Copy link
Contributor

@schmidt-sebastian schmidt-sebastian left a comment

Choose a reason for hiding this comment

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

LGTM (based on trust and some spot checking). Please fix remaining lint and build problems.

module.exports = function (config) {
const karmaConfig = Object.assign({}, karmaBase, {
// files to load into karma
files: files,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: This can be simplified.

packages/database/rollup.config.js Outdated Show resolved Hide resolved
@Feiyang1 Feiyang1 added this to the v9 GA milestone Aug 12, 2021
@Feiyang1 Feiyang1 added the v9 label Aug 12, 2021
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Aug 13, 2021

Binary Size Report

Affected SDKs

  • @firebase/database

    Type Base (4d747b1) Head (e807900) Diff
    browser 296 kB 247 kB -48.9 kB (-16.5%)
    main 299 kB 280 kB -19.2 kB (-6.4%)
    module 296 kB 247 kB -48.9 kB (-16.5%)
  • @firebase/database-compat

    Type Base (4d747b1) Head (e807900) Diff
    browser 86.6 kB 17.9 kB -68.7 kB (-79.4%)
    main 103 kB 22.9 kB -79.7 kB (-77.7%)
    module 86.6 kB 17.9 kB -68.7 kB (-79.4%)
  • firebase

    Type Base (4d747b1) Head (e807900) Diff
    firebase-database.js 188 kB 120 kB -68.0 kB (-36.2%)
    firebase.js 896 kB 836 kB -60.0 kB (-6.7%)

Test Logs

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Aug 13, 2021

Size Analysis Report

Affected Products

No changes between base commit (4d747b1) and head commit (e807900).

@Feiyang1 Feiyang1 changed the base branch from master to fei-v9-main August 13, 2021 01:04
@Feiyang1 Feiyang1 requested a review from zwu52 as a code owner August 18, 2021 06:40
@Feiyang1 Feiyang1 merged commit 568459d into fei-v9-main Aug 20, 2021
@Feiyang1 Feiyang1 deleted the fei-v9-main-database branch August 20, 2021 00:36
Feiyang1 added a commit that referenced this pull request Aug 24, 2021
* compile database

* pass database tests

* compile and test database-compat

* pass all tests

* prettier

* cleanup

* fix lint

* address comments

* what is going on with ci

* use correct case in import path

* uppercase

* rename

* fix component name
Feiyang1 added a commit that referenced this pull request Aug 24, 2021
* compile database

* pass database tests

* compile and test database-compat

* pass all tests

* prettier

* cleanup

* fix lint

* address comments

* what is going on with ci

* use correct case in import path

* uppercase

* rename

* fix component name
Feiyang1 added a commit that referenced this pull request Aug 24, 2021
* compile database

* pass database tests

* compile and test database-compat

* pass all tests

* prettier

* cleanup

* fix lint

* address comments

* what is going on with ci

* use correct case in import path

* uppercase

* rename

* fix component name
Feiyang1 added a commit that referenced this pull request Aug 24, 2021
* Split storage and storage-compat (#5271)

* separate storage-compat from storage

* commit

* fix tests

* wat?

* build storage-compat

* save

* fixing some compat tests

* format

* update import path

* format

* get compat tests to work

* format

* update package json

* cleanup

* format

* address comments

* Update packages/storage-compat/test/unit/service.test.ts

Co-authored-by: Christina Holland <chholland@google.com>

* add missing dev dep

* Update packages/storage-compat/package.json

* Update packages/storage-compat/package.json

* Update deps after v8 release

* update compat version

* Split database and database-compat (#5276)

* compile database

* pass database tests

* compile and test database-compat

* pass all tests

* prettier

* cleanup

* fix lint

* address comments

* what is going on with ci

* use correct case in import path

* uppercase

* rename

* fix component name

* Repo refactoring for v9 (#5325)

* rename folders

* rename folders

* migrate app and app-compat

* migrate installations and analytics

* migrate app check

* migrate auth

* migrate functions

* migrate messaging

* migrate performance

* migrate remoteconfig

* remove v8 code

* update versions

* migrate firebase

* fix merge issues

* save

* fix firebase builds

* update changeset config

* update build scripts

* update release script

* fix functions typings

* remove redundant typings

* update path

* treat external dependencies correctly

* fix lint

* remove firestore-compat references

* fix build issues

* update build scripts

* update pkg json

* fix test

* fix some tests

* fix some tests

* fix integration tests

* fixes

* fix dep

* update ci tests

* resolve todos

* remove exp references

* docgen build

* export FirebaseError (#5349)

* firebase packaging update (#5348)

* Fei v9 main firestore (#5319)

* compile firestore

* update typings path

* compile firestore compat

* lint compat

* make test work

* get most tests work

* build

* remove special paths

* update firestore-compat pkg json

* fix merge issues

* Simplify bundles

* Prettier

* Fix Bundle compile

* Fix build

* address comments

* console build

* Fix all tests

* Prettier

* fix merge issues

* fix typo

* update paths

* app-exp

* fix lint

* correct path

* fix

* fix compat lint

* fix firestore integration

* fix workflow

* update dep

* Always block on Auth (#5340)

* Always block on Auth

* Don't block on token if already recevied

* fix lint

* build firestore

* remove memory only tests

* fix firebase integration tests

* enable more build and tests

* add firestore-compat to firebase dep

* fix auth compat class

* enable auth test

* auth package doc

* Update API reports

Co-authored-by: Sebastian Schmidt <mrschmidt@google.com>
Co-authored-by: Feiyang1 <Feiyang1@users.noreply.github.com>

* rebasing

* Create registerMessagingCompat.ts

* Add changeset for v9 (#5350)

* add changeset

* publish messaging interop

* add firestore bumps

* update changeset

* Revert "Create registerMessagingCompat.ts"

This reverts commit 0b95a51.

* Revert "rebasing"

This reverts commit a8bf697.

* Update index.d.ts (#5355)

* Fix Context Check in `Messaging-Compat` (#5353)

* Add Rules Unit Testing v2 skeleton for v9 release (#5352)

* Reset package for next major version.

* Switch emulator script to node-fetch.

* Migrate the other request call to fetch.

* Update packages/rules-unit-testing/package.json

Co-authored-by: Feiyang <feiyangc@google.com>

* Create stale-ducks-live.md

* Revert version change.

Co-authored-by: Feiyang <feiyangc@google.com>

* fix import path (#5356)

* Add new types and function stubs for RUT vNext. (#5316)

* Add new types and function stubs.

* Fix types for testEnv.emulators.

* Add util functions.

* Add withFunctionTriggersDisabled overloads.

* Improve typing for EmulatorConfig.

* Fix tests.

* Rename test_environment.ts to initialize.ts.

* Add a dummy test to make CI pass.

* Implement emulators discovery in RUTv2. (#5334)

* Add new types and function stubs.

* Fix types for testEnv.emulators.

* Add util functions.

* Add withFunctionTriggersDisabled overloads.

* Improve typing for EmulatorConfig.

* Fix tests.

* Rename test_environment.ts to initialize.ts.

* Add a dummy test to make CI pass.

* Implement emulators discovery.

* Use URL object from global.

* Fix unreachable  error code.

* Implement most of RUTv2 features. (#5343)

* Implement most of RUTv2 features.

* Switch to compat instances.

* Remove legacy code.

* Use public typings for release (#5358)

* use public typings for storage

* use public typings for database

* Implement rest of RUTv2 features. (#5360)

* Implement loading rules and withFunctionTriggersDisabled.

* Implement clearFirestore and storage.

* Add missing await.

* Add default bucketUrl.

* Use alternative method to clear bucket.

* Use default param (review feedback).

* Storage typing updates (#5359)

* rename to StorageError

* more type safety

* Update API reports

* update api report

Co-authored-by: Feiyang1 <Feiyang1@users.noreply.github.com>

* correct component name

* fix installtions-compat typing

* remove rules-unit-testing from ignore list

* reenable things

* support mjs files

* correct matching pattern

* transform only @firebase/util

* use public types for database doc

* transform all but compat packages

Co-authored-by: Sebastian Schmidt <mrschmidt@google.com>
Co-authored-by: Feiyang1 <Feiyang1@users.noreply.github.com>
Co-authored-by: kai <zwu52@uw.edu>
Co-authored-by: Yuchen Shi <yuchenshi@google.com>
Co-authored-by: Kai Wu <zwu52@users.noreply.github.com>
@firebase firebase locked and limited conversation to collaborators Sep 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants