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

(Re)Remove _createDataStoreWithProps from dataStoreContext.ts #3631

Closed
heliocliu opened this issue Sep 14, 2020 · 7 comments
Closed

(Re)Remove _createDataStoreWithProps from dataStoreContext.ts #3631

heliocliu opened this issue Sep 14, 2020 · 7 comments
Labels
ado Issue is tracked in the internal Azure DevOps backlog api deprecation Changes to a deprecated API area: runtime Runtime related issues refactor Code refactoring and cleanup triage
Milestone

Comments

@heliocliu
Copy link
Contributor

heliocliu commented Sep 14, 2020

This API along with the supporting context props were removed in #2931 and #2951 but Bohemia was not prepared to receive these blessings in the 0.25 release and they needed to be added back in. We expect they might be ready by 0.27

@heliocliu heliocliu added the area: runtime Runtime related issues label Sep 14, 2020
@heliocliu heliocliu self-assigned this Sep 14, 2020
heliocliu added a commit that referenced this issue Sep 15, 2020
#3631
Minimally restore _createDataStoreWithProps. This includes adding back the API and adding back passing initial configs through the context. Bohemia isn't quite ready to move off this API yet but they will be soon™.

Future: port this change to 0.26 and main branches as well (Bohemia is not expected to be ready by 0.27 release)
heliocliu added a commit that referenced this issue Sep 17, 2020
…3662)

#3631
Port earlier change to minimally restore _createDataStoreWithProps to 0.26. Additional work in the port to 0.26 included adapting the change to the other component creation changes that happened between 0.25 and 0.26.

Future: bring this to main
heliocliu added a commit that referenced this issue Sep 17, 2020
#3631
Port earlier change to minimally restore _createDataStoreWithProps to main. Differences from 0.26 port include a minor merge conflict around a different added arg in LocalDataStoreContext ctor
@heliocliu heliocliu added this to the October 2020 milestone Sep 18, 2020
@curtisman curtisman added the refactor Code refactoring and cleanup label Oct 1, 2020
@heliocliu heliocliu modified the milestones: October 2020, Future Nov 2, 2020
@heliocliu
Copy link
Contributor Author

@leeviana is bohemia ready for the new create flow? is this something that can be done for november and consumed in 0.29?

@leeviana
Copy link
Contributor

leeviana commented Nov 9, 2020

We still set the rootcomponentconfig during initializingfirsttime. Since our office container now supports different root configs, we still allow for initialConfigs to be used for those components. We could go back to what we had years ago (IComponentWithInitialConfig) but is this really what is recommended for generic initial config values for all components?

@heliocliu
Copy link
Contributor Author

IComponentWithInitialConfig kind of sounds like a step backward? If more time is needed to get it right that's fine (especially since Vlad has more related changes in the pipe that may affect this); just wondering if this item is ready to do

@leeviana
Copy link
Contributor

IComponentWithInitialConfig is essentially what scriptor is hacking now but I think for this issue we only care about root component creations. @cristighr1995 does it make sense to take in root component config (canvas component type + initial config for canvas component) on some sort of Canvas related provider interface method instead of the initial config?

@ghost
Copy link

ghost commented Jun 8, 2021

This issue has been automatically marked as stale because it has had no activity for 180 days. It will be closed if no further activity occurs within 8 days of this comment. Thank you for your contributions to Fluid Framework!

@ssimic2
Copy link

ssimic2 commented Jan 3, 2022

Related: #8292

@ssimic2
Copy link

ssimic2 commented Jan 21, 2022

@heliocliu @vladsud can you help us clarify what the original intention was with this deprecation. @leeviana has shared her comments here: #8292, and that ticket would be covered with the solution that we determine here (around _createDataStoreWithProps ).

Partner repos seem to be relying on createDataStoreWithProps to inject initial config for their Data Objects (through datastore context). What is the alternative path here? Datastore creation paths leading through runtime are dealing with IFluidDataStoreFactory interface that provides no means of handing off this initial config.

@vladsud I have seen related changes here: #4163. It still seems that partners would need to deal directly with aqueduct/dataobjecy factory APIs to setup initProps.

@scottn12 scottn12 added the api deprecation Changes to a deprecated API label Jan 26, 2022
@curtisman curtisman added the ado Issue is tracked in the internal Azure DevOps backlog label Jun 20, 2022
@ghost ghost added the triage label Jun 20, 2022
@ssimic2 ssimic2 closed this as completed Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ado Issue is tracked in the internal Azure DevOps backlog api deprecation Changes to a deprecated API area: runtime Runtime related issues refactor Code refactoring and cleanup triage
Projects
None yet
Development

No branches or pull requests

6 participants