can-connect icon indicating copy to clipboard operation
can-connect copied to clipboard

`gotInstanceData` method wiring is missing from the data/callbacks behavior

Open pmgmendes opened this issue 8 years ago • 9 comments

How often can you reproduce it?

  • [x] Always
  • [ ] Sometimes
  • [ ] Rarely
  • [ ] Unable
  • [ ] I didn’t try

Description: Although the documentation can-connect references a callback named gotData this method is not being wired to getData in the data/callbacks pairs so it can be overwritten by other behaviors. The pairs object in can-connect/data/callbacks/callbacks.js is expected to also have

var pairs = {
  ...
  getData: "gotInstanceData",
  ...
};

pmgmendes avatar Jun 06 '17 10:06 pmgmendes

The reference you see to gotData on the main can-connect documentation index page is outdated. This will be corrected shortly with a major documentation update that is underway.

The docs for can-connect/data/callbacks here correctly show createdData, destroyedData, gotListData & updatedData. These are the only callbacks that expected to be part of this behavior at this time.

I'll report back if we have a timeline for adding a gotData callback to the data/callbacks behavior. In the meantime the best workaround would be to create your own behavior that wraps getData and does whatever transformation to the request or response you require.

nlundquist avatar Jun 06 '17 17:06 nlundquist

@pmgmendes or ... submit a pull request with it fixed and we can have a release out immediately :-)

justinbmeyer avatar Jun 06 '17 18:06 justinbmeyer

@justinbmeyer So it should be there? Yeah, I'll do it.

pmgmendes avatar Jun 06 '17 19:06 pmgmendes

yeah, should be there. Btw, what are you using it for?

justinbmeyer avatar Jun 06 '17 20:06 justinbmeyer

I'm accessing a resource modeled as /api/resource/{id} which returns a simple key value map

{ "keyA": "valueA", "keyB": "valueB", ... }

This is a resource that returns only static data from the server (but might change after new re-deploys) and I want to cache the response in localStorage but the returned map doesn't have a unique identifier. So I've created the instance type as a define map with the following props

{ 
id: "string",
data: {}
}

And in gotInstanceData I intend to do something like

gotInstanceData: (response, params) => {
  return {
    id: params.id
    data: response
  }
}

This way I can make use of the behaviors that use instanceStore too. It might appear that I could be better off using cacheRequests behavior but I still want to perform those requests to the server so I'm using fallThroughCache strategy.

pmgmendes avatar Jun 06 '17 21:06 pmgmendes

@justinbmeyer I've added the gotData getData pair to data/callbacks and also included the getDatain the method list for validation. Some tests in real-time behavior fail

Error: can-connect: Extending behavior "data/callbacks" found base behavior "real-time" was missing required properties: ["getData"]

Should real-time implement getData? I've done it but I'm struggling a bit to have the tests properly evolved to support getData. Help needed.

pmgmendes avatar Jun 06 '17 23:06 pmgmendes

For this use case, would parse-data do what you need?

Sent from my iPhone

On Jun 6, 2017, at 4:03 PM, Pedro Mendes [email protected] wrote:

I'm accessing a resource modeled as /api/resource/{id} which returns a simple key value map

{ "keyA": "valueA", "keyB": "valueB", ... } This is a resource that returns only static data from the server (but might change after new re-deploys) and I want to cache the response in localStorage but the returned map doesn't have a unique identifier. So I've created the instance type as define map with props

{ id: "string", data: {} } And in gotInstanceData I intend to do something like

gotInstanceData: (response, params) => { return { id: params.id data: response } } This way I can make use of the behaviors that use instanceStore too. It might appear that I could be better off using cacheRequests behavior but I still want to perform those requests to the server so I'm using fallThroughCache strategy.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

justinbmeyer avatar Jun 07 '17 00:06 justinbmeyer

I need the request params and parseData just provides the response data. In the meantime I was able to overwrite getData as suggested by @bmomberger-bitovi and @nlundquist in Gitter to attain the same purpose.

const customBehavior = function (baseConnection) {
  return {
    getData: function (params) {
      return baseConnection.getData(params).then(function (instanceData) {
        return {
          id: params.id,
          data: instanceData
        };
      });
    }
  };
};

Todo.connection = connect([ ..., dataUrl, customBehavior, dataParse, ...], {...});

pmgmendes avatar Jun 07 '17 00:06 pmgmendes

Good to hear @pmgmendes! If you want to open a PR for your branch I can help you resolve the problems you're having with the tests. I suspect the issue you're seeing is due to the test for the real-time behavior using a "stub" behavior that only implements getListData instead of the full DataInterface. Adding an empty function called getData to that stub behavior should fix the test.

nlundquist avatar Jun 07 '17 15:06 nlundquist