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

[Cache] attributes of records got missing #8519

Closed
Jopie01 opened this issue Mar 30, 2023 · 4 comments · Fixed by #8518
Closed

[Cache] attributes of records got missing #8519

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

Comments

@Jopie01
Copy link

Jopie01 commented Mar 30, 2023

With some changes and upgrade to alpha.15 I accidentally bypassed the Cache so I added the CacheHandler back into my requestManager service and now the cache is hit again. Because the response is JSON, I extended the Cache and converted the JSON into JSONAPI data so that the default JSONAPI cache consumes the data.

import Cache from '@ember-data/json-api';

class RPCCache extends Cache {
  constructor(...args) {
    super(...args);
  }

  // convert the JSON response into a JSONAPI response
  put(document) {
    let res = [];
    const method = document.request.data.method.split('.');
    document.content.forEach((item) => {
      res.push({
        id: item.id,
        type: method.slice(1, method.length - 1).join('.'),
        attributes: item,
      });
    });
    document.content.data = res;
    return super.put(...arguments);
  }
}
{{#each @model as |menu|}}
  {{menu.attributes.icon}} {{menu.attributes.name}}<br/>
{{/each}}

This worked prior to alpha.15. In my template I had to add the attributes key but that's ok. I also tested it with converting the response directly after the fetch but it doesn't make a difference.

BTW, also the instantiateRecord isn't called when doing a store.findAll(model) or store.query(model, options) so some wire seems to be broken.

@runspired
Copy link
Contributor

can you push this to the repo? I'll want to debug it

@Jopie01
Copy link
Author

Jopie01 commented Mar 30, 2023

can you push this to the repo? I'll want to debug it

Sorry for not mention it. I've already done that.

@runspired
Copy link
Contributor

Currently, the CacheHandler only instantiates records in the response documents for op codes that it knows about, I think this check needs to be loosened to being only for whether a hidden symbol is present, so that hydration is optional. What happened here is that while fixing some of the other issues you've hit I scoped down which requests the cache-handler attempts to "hydrate", and so bumping the version you now get back just the identifiers. Fix inbound.

I suspect there's also some more documentation to add here around where/how to do what kinds of work.

@runspired
Copy link
Contributor

Sorry for not mention it. I've already done that.

No worries, figured that out

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