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

gotInstanceData method wiring is missing from the data/callbacks behavior #301

Open
1 of 5 tasks
pmgmendes opened this issue Jun 6, 2017 · 9 comments
Open
1 of 5 tasks

Comments

@pmgmendes
Copy link

pmgmendes commented Jun 6, 2017

How often can you reproduce it?

  • 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",
  ...
};
@nlundquist
Copy link
Contributor

nlundquist commented Jun 6, 2017

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.

@justinbmeyer
Copy link
Contributor

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

@pmgmendes
Copy link
Author

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

@justinbmeyer
Copy link
Contributor

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

@pmgmendes
Copy link
Author

pmgmendes commented Jun 6, 2017

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
Copy link
Author

@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.

@justinbmeyer
Copy link
Contributor

justinbmeyer commented Jun 7, 2017 via email

@pmgmendes
Copy link
Author

pmgmendes commented Jun 7, 2017

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, ...], {...});

@nlundquist
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants