fix(data): update DataStore to send correct Control Messages when starting #12861

david-mcafee · 2024-01-18T19:45:02Z

Description of changes

Fixes a DataStore bug where starting DataStore offline in a React Native app causes DataStore to hang indefinitely (and not work).

The control messages were originally updated here when removing the Node check, and reworked here to get DataStore tests passing (however, this introduced the above-mentioned bug).

This PR:

Environment	Control Message
browser / RN / et al	`SYNC_ENGINE_STORAGE_SUBSCRIBED`
Node	`SYNC_ENGINE_SYNC_QUERIES_READY`

Before this PR:

Environment	Control Message
browser	`SYNC_ENGINE_STORAGE_SUBSCRIBED`
Node / RN / et al	`SYNC_ENGINE_SYNC_QUERIES_READY`

Issue #, if available

#12776

Description of how you validated changes

Checklist

PR description included
yarn test passes
Tests are changed or added
Relevant documentation is changed or added (and PR referenced)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…rting

svidgen

Thanks, David. A few small suggestion if they make sense to you and can be done quickly. Otherwise, let's just get this shipped and into customers' hands ASAP!

packages/datastore/src/datastore/datastore.ts

svidgen · 2024-01-18T20:53:56Z

Thanks, David. A few small suggestion if they make sense to you and can be done quickly. Otherwise, let's just get this shipped and into customers' hands ASAP!

Oh, I didn't see we had failing tests now. That's strange, this fix makes a ton of sense. Maybe the tests are just spoofing environmental stuff assuming the old util is in place.

david-mcafee · 2024-01-18T21:14:47Z

Thanks, David. A few small suggestion if they make sense to you and can be done quickly. Otherwise, let's just get this shipped and into customers' hands ASAP!

Oh, I didn't see we had failing tests now. That's strange, this fix makes a ton of sense. Maybe the tests are just spoofing environmental stuff assuming the old util is in place.

Yep, that was the issue - isNode and isBrowser were both returning true in a test environment. I've updated the check so the tests will pass, just revalidating that change against my React Native sample.

packages/datastore/src/datastore/datastore.ts

* chore(auth): debounce refreshAuthTokens (#12845) * chore: add debounce callback helper * chore: add unit tests * chore: debounce fetchAuthSession * chore: fix unit test * chore: fix bundle size limits * chore: update debounce logic * chore: update dedup logic * chore: debounce refreshAuthTokens * chore: fix bundle size * chore: address feedback * chore: fix unit test * chore: address feedback * chore: update yarn.lock * chore: address feedbak * fix(data): update DataStore to send correct Control Messages when starting (#12861) * chore(release): Publish [ci skip] - @aws-amplify/adapter-nextjs@1.0.13 - @aws-amplify/analytics@7.0.13 - @aws-amplify/api@6.0.13 - @aws-amplify/api-graphql@4.0.13 - @aws-amplify/api-rest@4.0.13 - @aws-amplify/auth@6.0.13 - aws-amplify@6.0.13 - @aws-amplify/core@6.0.13 - @aws-amplify/datastore@5.0.13 - @aws-amplify/datastore-storage-adapter@2.1.13 - @aws-amplify/geo@3.0.13 - @aws-amplify/interactions@6.0.13 - @aws-amplify/notifications@2.0.13 - @aws-amplify/predictions@6.0.13 - @aws-amplify/pubsub@6.0.13 - @aws-amplify/react-native@1.0.13 - @aws-amplify/react-native-example@0.0.14 - @aws-amplify/rtn-push-notification@1.2.13 - @aws-amplify/rtn-web-browser@1.0.13 - @aws-amplify/storage@6.0.13 - tsc-compliance-test@0.1.13 * chore(release): update API docs [ci skip] * chore: enable codecov (#12876) * docs(auth,analytics): adding in-line docs for public apis (#12882) * docs(auth,analytics): adding in-line docs for public apis * fix: unblock the max length lint * chore(react-native): use react-native 0.71.0 as the dev dep * fix: Default branch resolution when running E2E tests (#12910) * fix: Lambda auth config value (#12922) * chore: renable caching the package list in GH actions * chore: remove unused codecov package (has been deprecated) * feat(api): expose HTTP response from API errors (#12835) --------- Co-authored-by: Hui Zhao <10602282+HuiSF@users.noreply.github.com> Co-authored-by: Jim Blanchard <jim.l.blanchard@gmail.com> * chore: temporarily disable codecov GH action integration (#12928) * ci: run flaky data e2es without retry (#12758) * ci: run flaky data e2es without retry * only run on chrome * restore workflow --------- Co-authored-by: Aaron S <94858815+stocaaro@users.noreply.github.com> * fix(core): Amplify.configure dispatches Hub event with unparsed config object (#12930) * chore(repo): use typescript 5.0.2 across workspace * chore(repo): refactor tsconfig hierarchy - move the base tsconfig into the root of the workspace - unifying tsconfig tsconfig.build tsconfig.test and tsconfig.watch settings * chore(repo): promote rollup dependencies into workspace - remove unused build.js script * chore(repo): add build:watch using rollup * chore(repo): setup eslint * chore(adapter-nextjs): migrate to eslint * fix(repo): test-github-actions using js-yaml already removed API * chore(repo): add Code Spell Checker to recommended extension list * chore(repo): add formatOn actions to formattingToggle controlables * fix: not appending notification configs * chore(release): Publish [ci skip] - @aws-amplify/adapter-nextjs@1.0.14 - @aws-amplify/analytics@7.0.14 - @aws-amplify/api@6.0.14 - @aws-amplify/api-graphql@4.0.14 - @aws-amplify/api-rest@4.0.14 - @aws-amplify/auth@6.0.14 - aws-amplify@6.0.14 - @aws-amplify/core@6.0.14 - @aws-amplify/datastore@5.0.14 - @aws-amplify/datastore-storage-adapter@2.1.14 - @aws-amplify/geo@3.0.14 - @aws-amplify/interactions@6.0.14 - @aws-amplify/notifications@2.0.14 - @aws-amplify/predictions@6.0.14 - @aws-amplify/pubsub@6.0.14 - @aws-amplify/react-native@1.0.14 - @aws-amplify/react-native-example@0.0.15 - @aws-amplify/rtn-push-notification@1.2.14 - @aws-amplify/rtn-web-browser@1.0.14 - @aws-amplify/storage@6.0.14 - tsc-compliance-test@0.1.14 * Revert "chore(repo): use typescript 5.0.2 across workspace" (#12941) * Revert "chore(repo): use typescript 5.0.2 across workspace" This reverts commit e20782e. * chore(api-graphql): temporarily install rollup to package * fix(repo): rollup generated sourcemap has wrong src path (#12947) * fix(datastore): Treat head as optional to avoid errors (#12936) * fix(datastore): Treat head as optional to avoid errors * Add testing and fix follow-on issue * fix(api-graphql): wrong arguments for GET operation of a CPK model * fix: Remove special e2e treatment for validated tests (#12946) fix: Remove special integ treament for validated tests * fix(datastore): Limit docs integ test to one browser to improve stability (#12937) * fix(api-graphql): same results returned for queries on the same model with different selection set - caused by the incomplete GraphQL documents caching * Update branch to work with merged updates from main --------- Co-authored-by: israx <70438514+israx@users.noreply.github.com> Co-authored-by: David McAfee <mcafd@amazon.com> Co-authored-by: Jim Blanchard <jim.l.blanchard@gmail.com> Co-authored-by: aws-amplify-bot <aws@amazon.com> Co-authored-by: Hui Zhao <10602282+HuiSF@users.noreply.github.com> Co-authored-by: AllanZhengYP <zheallan@amazon.com> Co-authored-by: Hui Zhao <zhohz@amazon.com> Co-authored-by: Ivan Artemiev <29709626+iartemiev@users.noreply.github.com> Co-authored-by: Aaron S <94858815+stocaaro@users.noreply.github.com> Co-authored-by: ManojNB <manojnb95@gmail.com> Co-authored-by: ManojNB <manojnb@amazon.com> Co-authored-by: Francisco Rodriguez <frodriguez.cs@gmail.com>

david-mcafee force-pushed the 12776 branch from c839c3b to 5dedae9 Compare January 18, 2024 19:45

david-mcafee changed the title ~~fix(data): update DataStore to send correct Control Messages when sta…~~ fix(data): update DataStore to send correct Control Messages when starting in non-browser environments Jan 18, 2024

david-mcafee force-pushed the 12776 branch from 5dedae9 to 5890d6f Compare January 18, 2024 19:46

fix(data): update DataStore to send correct Control Messages when sta…

f683bdc

…rting

david-mcafee force-pushed the 12776 branch from 5890d6f to f683bdc Compare January 18, 2024 19:51

david-mcafee changed the title ~~fix(data): update DataStore to send correct Control Messages when starting in non-browser environments~~ fix(data): update DataStore to send correct Control Messages when starting Jan 18, 2024

david-mcafee marked this pull request as ready for review January 18, 2024 19:58

david-mcafee requested a review from a team as a code owner January 18, 2024 19:58

david-mcafee requested review from elorzafe and HuiSF January 18, 2024 19:58

david-mcafee self-assigned this Jan 18, 2024

david-mcafee linked an issue Jan 18, 2024 that may be closed by this pull request

Amplify Datastore not working when I start app without internet #12776

Closed

3 tasks

Merge branch 'main' into 12776

105f2e9

svidgen previously approved these changes Jan 18, 2024

View reviewed changes

packages/datastore/src/datastore/datastore.ts Outdated Show resolved Hide resolved

packages/datastore/src/datastore/datastore.ts Outdated Show resolved Hide resolved

unit test fix

8b5db26

david-mcafee dismissed svidgen’s stale review via 8b5db26 January 18, 2024 21:06

david-mcafee requested a review from a team as a code owner January 18, 2024 21:06

svidgen reviewed Jan 18, 2024

View reviewed changes

packages/datastore/src/datastore/datastore.ts Outdated Show resolved Hide resolved

david-mcafee requested a review from svidgen January 18, 2024 22:24

david-mcafee added 2 commits January 18, 2024 19:17

fix tests

c1dfc52

test and util update

bdd45a0

david-mcafee force-pushed the 12776 branch from 65e7ff2 to bdd45a0 Compare January 19, 2024 05:22

add license

1980822

svidgen approved these changes Jan 19, 2024

View reviewed changes

stocaaro approved these changes Jan 19, 2024

View reviewed changes

david-mcafee merged commit 551a58c into main Jan 19, 2024
31 checks passed

david-mcafee deleted the 12776 branch January 19, 2024 18:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(data): update DataStore to send correct Control Messages when starting #12861

fix(data): update DataStore to send correct Control Messages when starting #12861

david-mcafee commented Jan 18, 2024 •

edited

svidgen left a comment

svidgen commented Jan 18, 2024

david-mcafee commented Jan 18, 2024

fix(data): update DataStore to send correct Control Messages when starting #12861

fix(data): update DataStore to send correct Control Messages when starting #12861

Conversation

david-mcafee commented Jan 18, 2024 • edited

Description of changes

This PR:

Before this PR:

Issue #, if available

Description of how you validated changes

Checklist

svidgen left a comment

Choose a reason for hiding this comment

svidgen commented Jan 18, 2024

david-mcafee commented Jan 18, 2024

david-mcafee commented Jan 18, 2024 •

edited