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

[Store] Documentation should tell something about store lifetimes #8510

Closed
Jopie01 opened this issue Mar 28, 2023 · 9 comments · Fixed by #8511 or #8517
Closed

[Store] Documentation should tell something about store lifetimes #8510

Jopie01 opened this issue Mar 28, 2023 · 9 comments · Fixed by #8511 or #8517
Labels
🏷️ bug This PR primarily fixes a reported issue 🏷️ doc This PR adds/improves/or fixes documentation

Comments

@Jopie01
Copy link

Jopie01 commented Mar 28, 2023

Sorry for flooding with issues 😢 But hopefully it will help hammering out the nasty details.

When requesting data through the store you will get the error

Error while processing route: index store.lifetimes is undefined calcShouldFetch

I found the function here: https://github.com/emberjs/data/blob/main/packages/store/src/-private/cache-handler.ts#L68 and in particular https://github.com/emberjs/data/blob/main/packages/store/src/-private/cache-handler.ts#L15

So I added in the constructor of my route

constructor(...args) {
  super(...args);
  this.store.lifetimes = {
    isHardExpired() { return true; }
  }
}

Which solves the error but to me this basically disables the cache right? So maybe in the documentation the lifetimes of the store can be explained in more detail?

@runspired
Copy link
Contributor

ah, that is supposed to be optional. Will fix :)

some limited documentation for this does exist (and I've updated the RFC with it too here emberjs/rfcs#919) but since we don't publish canary/beta api docs anywhere right now it's only viewable in the source code here:

/**
* A Property which an App may set to provide a Lifetimes Service
* to control when a cached request becomes stale.
*
* ```ts
* store.lifetimes = {
* // make the request and ignore the current cache state
* isHardExpired() {}
*
* // make the request in the background if true, return cache state
* isSoftExpired() {}
* }
* ```
*
* @public
* @property {LivetimesService|undefined} lifetimes
*/
declare lifetimes?: LifetimesService;

isHardExpired is basically equivalent to shouldReloadRecord and shouldReloadAll hooks on the adapters but now for all requests not just findAll and findRecord

isSoftExpired is basically equivalent to shouldBackgroundReloadRecord and shouldBackgroundReloadAll hooks on the adapters, but similarly more expansive in what they can cover.

My hope is that once the initial implementation "settles" (e.g. all current EmberData paradigms integrate with the RequestManager and upgraded cache in a first-class way) that we'll offer something you can plugin to ensure a sensible default lifetime based on either meta, a number of milliseconds in your config, e-tags in the response headers, or even by parsing information out of the cache header.

@runspired
Copy link
Contributor

Sorry for flooding with issues 😢 But hopefully it will help hammering out the nasty details.

It is in fact hugely appreciated, I very - very - very rarely get feedback on alpha and beta cycles. In fact the lack of this feedback is part of why EmberData generally does not do beta cycles (hence 4.12 being alpha currently even though it will release next week), because otherwise the length of time between implementation and feedback leads to a very slow dev-cycle with a great deal of context-loss and higher number of patches.

@Jopie01
Copy link
Author

Jopie01 commented Mar 29, 2023

that is supposed to be optional.

I upgraded to alpha.14 but couldn't remove my code which set the lifetimes from the application route 😢

I don't know the default settings which are in use now by ember-data but could these somehow added as default?

@runspired
Copy link
Contributor

Going to reopen until I figure out what's up. Broadly speaking, you shouldn't ever need to set lifetimes but you should need to do the same configuration as is in ember-data if you are using ember-data but extending @ember-data/store instead of ember-data/store. This distinction is ... annoying but necessary, especially if we want to give you the ability to use the same request-manager as a separate service.

@runspired runspired reopened this Mar 29, 2023
@Jopie01
Copy link
Author

Jopie01 commented Mar 29, 2023

To be crystal clear, I'm extending @ember-data/store in this new system. It that the right one?

import Store, { CacheHandler } from '@ember-data/store';
......

export default class StoreService extends Store {
  ....
}

@runspired
Copy link
Contributor

@Jopie01 I think for your use case it is, but it means you've got to mirror the setup that ember-data/store would have done. If you're on discord mind finding me and giving me a call? It might help me fix a few things faster here.

@Jopie01
Copy link
Author

Jopie01 commented Mar 29, 2023

but it means you've got to mirror the setup that ember-data/store would have done.

Not much to see in https://github.com/emberjs/data/blob/15049beed153f1375b3eb2bfd9ba31da744b7e14/packages/-ember-data/addon/-private/index.ts which I'm not doing already. But can it be that 'legacy' never reaches a function where store.lifetimes is used?
What I also discovered was that setting this.store.lifetimes also fixes #8508 (removing setting the lifetimes will also bring back #8508)

If you're on discord

Sorry I don't have discord, but if it helps I can send the code (after cleaning up)

@runspired
Copy link
Contributor

Yeah access to the repo / at least the store and request manager setup bits would be helpful

@Jopie01
Copy link
Author

Jopie01 commented Mar 29, 2023

Yeah access to the repo / at least the store and request manager setup bits would be helpful

Just pushed everything and added you as collaborator.

But I think you will end up doing something similar as ember-simple-auth with the this.session.setup() in the application route. The 'old' ember-data is initialized by the Ember app right? So at the start the store is there, but in the new situation the store is lazily created, basically on the first request.

@runspired runspired added 🏷️ bug This PR primarily fixes a reported issue 🏷️ doc This PR adds/improves/or fixes documentation and removed Bug labels Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bug This PR primarily fixes a reported issue 🏷️ doc This PR adds/improves/or fixes documentation
Projects
None yet
2 participants