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

feat: Add instance data in loadInstanceOptionsFromStack #1380

Merged
merged 1 commit into from
Sep 1, 2023

Conversation

zatteo
Copy link
Contributor

@zatteo zatteo commented Aug 31, 2023

We need locale attribute on flagship app.

@zatteo zatteo force-pushed the feat/add-instance-load-instance-options-from-stack branch from 26a726c to c897619 Compare August 31, 2023 12:43
Copy link
Contributor

@paultranvan paultranvan left a comment

Choose a reason for hiding this comment

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

The doc needs to be updated, but looks good!

@zatteo zatteo force-pushed the feat/add-instance-load-instance-options-from-stack branch 2 times, most recently from a55057b to 1c62657 Compare August 31, 2023 13:29
@zatteo
Copy link
Contributor Author

zatteo commented Aug 31, 2023

Related #1381

@zatteo zatteo force-pushed the feat/add-instance-load-instance-options-from-stack branch 2 times, most recently from 932276f to 08c75c3 Compare August 31, 2023 14:33
@@ -1685,8 +1685,14 @@ instantiation of the client.`
Q('io.cozy.settings').getById('io.cozy.settings.capabilities')
)

const { data: instanceData } = await this.query(
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be getOrFechFromState (I don’t know the name exactly) and you should add a fetchPolicy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. It should be always fresh so no fetchPolicy no?

Copy link
Contributor

Choose a reason for hiding this comment

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

@zatteo I think using fetchQueryAndGetFromState as proposed by @Crash-- is only useful if we add a fetchPolicy. That being said, I'm not sure we want this kind of logic at this level, I don't think I like the idea of a forced fetchPolicy that could lead to unexpected results from the caller.
In any case, the behaviour should be the same for the other query just above on io.cozy.settings.capabilities.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So :

  • no fetchPolicy because we need it fresh
  • same method for the two calls : decided to stay on query for the moment

@zatteo zatteo force-pushed the feat/add-instance-load-instance-options-from-stack branch from 08c75c3 to 367e354 Compare August 31, 2023 15:12
@zatteo zatteo force-pushed the feat/add-instance-load-instance-options-from-stack branch from 367e354 to 1bfc6a0 Compare September 1, 2023 13:07
@zatteo zatteo merged commit 3f3b0d8 into master Sep 1, 2023
4 checks passed
@zatteo zatteo deleted the feat/add-instance-load-instance-options-from-stack branch September 1, 2023 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants