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.

Sign up for GitHub

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

Jump to bottom

Remove _createDataStoreWithProps #22996

Merged
merged 13 commits into from
Jan 8, 2025
Merged

Remove _createDataStoreWithProps #22996

merged 13 commits into from
Jan 8, 2025

Conversation

tyler-cai-microsoft
Copy link
Contributor

@tyler-cai-microsoft tyler-cai-microsoft commented Nov 6, 2024 •
edited
Loading

AB#856
#22993

Description

Removes APIs that have been deprecated. We have tried to move away from _createDataStoreWithProps as this causes collisions when creating live datastores. Solutions involved aliasing. The original issue with the removal was that there was no way to put in props. Added the initialState parameter to PureDataObjectFactory.createInstanceWithDataStore as pass in props.

Remove APIs

  • IContainerRuntimeBase._createDataStoreWithProps
  • ContainerRuntime._createDataStoreWithProps

Alternatives

  • PureDataObjectFactory.createInstanceWithDataStore allows props to be passed in via the initialState parameter.

Merge Notes

  • Merge only after main is ready for 2.20 changes.

@tyler-cai-microsoft tyler-cai-microsoft requested review from a team as code owners November 6, 2024 00:28
@github-actions github-actions bot added area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct area: runtime Runtime related issues area: tests Tests to add, test infrastructure improvements, etc changeset-present public api change Changes to a public API base: main PRs targeted against main branch labels Nov 6, 2024
@tyler-cai-microsoft tyler-cai-microsoft mentioned this pull request Nov 6, 2024
Closed
@tyler-cai-microsoft tyler-cai-microsoft changed the title Legacy breaks/client/2.20/remove create data object with props Remove _createDataStoreWithProps Nov 6, 2024
tylerbutler
tylerbutler approved these changes Nov 6, 2024
View reviewed changes
Copy link
Member

@tylerbutler tylerbutler left a comment

Choose a reason for hiding this comment

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

Approving for docs; had some rewording and clarification that would be good to add.

.changeset/gold-maps-cut.md Outdated Show resolved Hide resolved
.changeset/gold-maps-cut.md Outdated Show resolved Hide resolved
.changeset/gold-maps-cut.md Outdated Show resolved Hide resolved
.changeset/gold-maps-cut.md Show resolved Hide resolved
msfluid-bot
msfluid-bot reviewed Nov 6, 2024
View reviewed changes
Copy link
Collaborator

@msfluid-bot msfluid-bot left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Code Coverage Summary

↑ packages.runtime.container-runtime.src:
 Branch Coverage Change: 0.05%
Metric NameBaseline coveragePR coverageCoverage Diff
Branch Coverage 93.00% 93.05% ↑ 0.05%

Baseline commit: 62836c0
Baseline build: 316051
Happy Coding!!

Code coverage comparison check passed!!

@msfluid-bot
Copy link
Collaborator

msfluid-bot commented Nov 6, 2024 •
edited
Loading

⯆ @fluid-example/bundle-size-tests: -511 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 470.37 KB 470.22 KB ⯆ -154 Bytes
azureClient.js 567.1 KB 566.97 KB ⯆ -140 Bytes
connectionState.js 724 Bytes 724 Bytes ■ No change
containerRuntime.js 266.24 KB 266.07 KB ⯆ -175 Bytes
fluidFramework.js 358.47 KB 358.48 KB ⯅ +14 Bytes
loader.js 134.2 KB 134.21 KB ⯅ +14 Bytes
map.js 42.68 KB 42.69 KB ⯅ +7 Bytes
matrix.js 150.36 KB 150.36 KB ⯅ +7 Bytes
odspClient.js 534.57 KB 534.43 KB ⯆ -140 Bytes
odspDriver.js 99.49 KB 99.51 KB ⯅ +21 Bytes
odspPrefetchSnapshot.js 43.04 KB 43.06 KB ⯅ +14 Bytes
sharedString.js 166.51 KB 166.52 KB ⯅ +7 Bytes
sharedTree.js 348.96 KB 348.96 KB ⯅ +7 Bytes
Total Size 3.26 MB 3.26 MB ⯆ -511 Bytes

Baseline commit: 62836c0

Generated by 🚫 dangerJS against 6b41513

@jason-ha jason-ha linked an issue Dec 11, 2024 that may be closed by this pull request
Remove _createDataStoreWithProps #22993
Closed
jatgarg
jatgarg reviewed Jan 8, 2025
View reviewed changes
.changeset/gold-maps-cut.md
"@fluidframework/container-runtime-definitions": minor
"@fluidframework/datastore": minor
"@fluidframework/runtime-definitions": minor
"@fluidframework/test-runtime-utils": minor
Copy link
Contributor

Choose a reason for hiding this comment

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

@alexvy86 I guess we should only add package from the API is removed and not all packages which are in the PR?
cc: @tylerbutler

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 8, 2025

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output


> [email protected] ci:check-links /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test "npm run serve -- --no-open" 3000 check-links

1: starting server using command "npm run serve -- --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"


> [email protected] serve
> docusaurus serve --no-open

[SUCCESS] Serving "build" directory at: http://localhost:3000/

> [email protected] check-links
> linkcheck http://localhost:3000 --skip-file skipped-urls.txt

Crawling...

Stats:
  170492 links
    1603 destination URLs
    1838 URLs ignored
       0 warnings
       0 errors

@tyler-cai-microsoft tyler-cai-microsoft merged commit bd243fb into main Jan 8, 2025
31 checks passed
@tyler-cai-microsoft tyler-cai-microsoft deleted the legacy-breaks/client/2.20/remove_createDataObjectWithProps branch January 8, 2025 23:37
@github-actions github-actions bot mentioned this pull request Jan 8, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msfluid-bot msfluid-bot msfluid-bot left review comments

@jatgarg jatgarg jatgarg approved these changes

@tylerbutler tylerbutler tylerbutler approved these changes

Assignees
No one assigned
Labels
area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct area: runtime Runtime related issues area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch changeset-present public api change Changes to a public API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove _createDataStoreWithProps
4 participants
@tyler-cai-microsoft @msfluid-bot @tylerbutler @jatgarg