Skip to content

Firestore: Deduplicate some repeated logic in OfflineComponentProvider #7952

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

dconeybe
Copy link
Contributor

@dconeybe dconeybe commented Jan 16, 2024

This PR simply deduplicates some logic in the OfflineComponentProvider class that starts and stops the schedulers (the gc scheduler and index backfiller scheduler). This small cleanup will be built upon in the upcoming PR that makes the index backfiller scheduler tree-shakeable (#7902).

Work In Progress

This PR is a work-in-progress. As of Jan 22, 2024, work on this PR has been de-prioritized but I'm leaving it here so it can be picked up in the future. Googlers see b/293449522 for details.

…ions that start and stop the gcScheduler and indexBackfillerScheduler.
@dconeybe dconeybe self-assigned this Jan 16, 2024
Copy link

changeset-bot bot commented Jan 16, 2024

🦋 Changeset detected

Latest commit: b103153

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Not sure what this means? Click here to learn what changesets are.

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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jan 16, 2024

Size Report 1

Affected Products

  • @firebase/firestore

    TypeBase (49c7903)Merge (fdf4084)Diff
    browser375 kB375 kB-219 B (-0.1%)
    esm5360 kB360 kB-169 B (-0.0%)
    main577 kB577 kB-59 B (-0.0%)
    module375 kB375 kB-219 B (-0.1%)
    react-native375 kB375 kB-219 B (-0.1%)
  • bundle

    TypeBase (49c7903)Merge (fdf4084)Diff
    firestore (CSI Auto Indexing Disable and Delete)268 kB268 kB-83 B (-0.0%)
    firestore (CSI Auto Indexing Enable)268 kB268 kB-83 B (-0.0%)
    firestore (Persistence)303 kB302 kB-219 B (-0.1%)
    firestore (Read Write w Persistence)321 kB321 kB-219 B (-0.1%)
  • firebase

    TypeBase (49c7903)Merge (fdf4084)Diff
    firebase-compat.js779 kB779 kB-219 B (-0.0%)
    firebase-firestore-compat.js341 kB341 kB-219 B (-0.1%)
    firebase-firestore.js434 kB434 kB-219 B (-0.1%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/kzjRL5NQOo.html

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jan 16, 2024

Size Analysis Report 1

Affected Products

  • @firebase/firestore

    • enableIndexedDbPersistence

      Size

      TypeBase (49c7903)Merge (fdf4084)Diff
      size186 kB186 kB-83 B (-0.0%)
      size-with-ext-deps258 kB258 kB-83 B (-0.0%)
    • enableMultiTabIndexedDbPersistence

      Size

      TypeBase (49c7903)Merge (fdf4084)Diff
      size222 kB222 kB-219 B (-0.1%)
      size-with-ext-deps294 kB294 kB-219 B (-0.1%)
    • persistentLocalCache

      Size

      TypeBase (49c7903)Merge (fdf4084)Diff
      size183 kB183 kB-83 B (-0.0%)
      size-with-ext-deps255 kB255 kB-83 B (-0.0%)
    • persistentMultipleTabManager

      Size

      TypeBase (49c7903)Merge (fdf4084)Diff
      size219 kB218 kB-219 B (-0.1%)
      size-with-ext-deps291 kB291 kB-219 B (-0.1%)
    • persistentSingleTabManager

      Size

      TypeBase (49c7903)Merge (fdf4084)Diff
      size183 kB183 kB-83 B (-0.0%)
      size-with-ext-deps255 kB255 kB-83 B (-0.0%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/4uJzpkUkjR.html

@dconeybe dconeybe marked this pull request as ready for review January 16, 2024 06:52
@dconeybe dconeybe requested review from a team as code owners January 16, 2024 06:52
@dconeybe dconeybe requested a review from wu-hui January 16, 2024 06:53
…AGE to $GITHUB_OUTPUT instead of echo since embedded quotes in the error message cause the shell to mis-interpret them

For example, in https://github.com/firebase/firebase-js-sdk/actions/runs/7586489122/job/20664769245?pr=7952 the following error occurred, which this commit fixes:

```
$ /home/runner/work/firebase-js-sdk/firebase-js-sdk/node_modules/.bin/ts-node-script scripts/ci/check_changeset.ts
/home/runner/work/firebase-js-sdk/firebase-js-sdk/node_modules/child-process-promise/lib/index.js:33
            var cpError = new ChildProcessError(err.message, err.code, child_process, stdout, stderr);
                          ^
ChildProcessError: Command failed: echo "CHANGESET_ERROR_MESSAGE=- Changeset formatting error in following file:%0A    \`\`\`%0A    Some packages have been changed but no changesets were found. Run \`changeset add\` to resolve this error.%0A    If this change doesn't need a release, run \`changeset add --empty\`.%0A    \`\`\`%0A" >> $GITHUB_OUTPUT
/bin/sh: 1: Syntax error: Unterminated quoted string
 \`echo "CHANGESET_ERROR_MESSAGE=- Changeset formatting error in following file:%0A    \`\`\`%0A    Some packages have been changed but no changesets were found. Run \`changeset add\` to resolve this error.%0A    If this change doesn't need a release, run \`changeset add --empty\`.%0A    \`\`\`%0A" >> $GITHUB_OUTPUT\` (exited with error code 2)
    at callback (/home/runner/work/firebase-js-sdk/firebase-js-sdk/node_modules/child-process-promise/lib/index.js:33:27)
    at ChildProcess.exithandler (node:child_process:410:5)
    at ChildProcess.emit (node:events:513:28)
    at ChildProcess.emit (node:domain:489:12)
    at maybeClose (node:internal/child_process:1100:16)
    at Socket.<anonymous> (node:internal/child_process:458:11)
    at Socket.emit (node:events:513:28)
    at Socket.emit (node:domain:489:12)
    at Pipe.<anonymous> (node:net:301:12) {
  code: 2,
  childProcess: {
    _forkChild: [Function: _forkChild],
    ChildProcess: [Function: ChildProcess],
    exec: [Function: exec],
    execFile: [Function: execFile],
    execFileSync: [Function: execFileSync],
    execSync: [Function: execSync],
    fork: [Function: fork],
    spawn: [Function: spawn],
    spawnSync: [Function: spawnSync]
  },
  stdout: '',
  stderr: '/bin/sh: 1: Syntax error: Unterminated quoted string\n'
}
error Command failed with exit code 1.
```
Copy link
Contributor

github-actions bot commented Jan 19, 2024

Changeset File Check ⚠️

  • Warning: This PR modifies files in the following packages but they have not been included in the changeset file:%0A - @firebase/firestore%0A%0A Make sure this was intentional.

@dconeybe dconeybe requested a review from a team as a code owner January 19, 2024 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants