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

possible breaking changes for 6.x #38

Open
16 tasks
devvmh opened this issue Aug 9, 2016 · 1 comment
Open
16 tasks

possible breaking changes for 6.x #38

devvmh opened this issue Aug 9, 2016 · 1 comment

Comments

@devvmh
Copy link
Owner

devvmh commented Aug 9, 2016

Collect desired breaking changes that if merged would trigger a version bump to 6.x. More may still be added:

Integrate normalizr

This should, if done right, solve issue #6, #7, and #19 all together. This library originally assumed all FETCH responses came wrapped in a data envelope, but that assumption can't remain. Currently there is a workaround active in the master branch, but normalizr offers a more robust and recognizable solution to this problem.

To implement this, the following changes are needed:

  • Add normalizr as a dependency (probably) or a peerDependency (ideally)
  • Change FETCH action creators to take a Schema object
  • Change the saga to store the Schema object, and then denormalize the data. After that, it can dispatch one action for each collection. There will need to be a new action type (GOT_NORMALIZED_DATA), since FETCH_ONE_SUCCESS would require dispatching potentially a lot of actions, but FETCH_SUCCESS would store collections and parameters, which doesn't make sense.
  • Change the reducer to handle GOT_NORMALIZED_DATA
  • Investigate further to see if it's helpful to store the Schema objects in the store at any point (currently I think this isn't helpful)
  • Scrub references to id, using the schema's idAttribute instead.
  • Investigate FETCH_ONE, CREATE, and UPDATE to see how normalizr will be implemented
  • Investigate DELETE to see if normalizr needs to be implemented.

Handle response envelopes in a smarter way

Once normalizr is in, we'll be in a better spot to handle response envelopes. Currently there are two big issues with how it's handled.

  1. 'data' is assumed as the key holding the data, and if it's not present the payload is assumed to be the records.
  2. The response envelope, if it exists, is stored in the awkwardly named 'otherInfo'.

I would like to do the following:

  • Use normalizr Schema to specify the shape of the response.
  • Specify a clever default that emulates the current behaviour as much as possible.
  • Store the response envelope, if it exists, in record.envelope (instead of record.otherInfo).
  • Deprecate record.otherInfo and remove it from documentation, but leave it as a key until 7.x or even 8.x (or forever, if 6.x is just so stable and great we don't need any more breaking changes).

Store response headers, and make them accessible

I really love http://www.vinaysahni.com/best-practices-for-a-pragmatic-restful-api, and it inspired the backend that this library was originally written to consume. In this document, he talks about how pagination may someday soon move to live in response headers, rather than in a data envelope. I'd love to support that. It will involve a breaking change to ApiClient.js however *(and by extension, any custom ApiClient implementations that library consumers have written).

  • Update ApiClient.js to return headers as well as response body. If possible, continue to handle responses that don't include the headers. I think the best way will be to return an object with the shape { headers, payload }, and if the return value isn't in that shape, assume it is a 5.x ApiClient that is only passing a payload.
  • Store the headers alongside the envelope in the collections store, and make them accessible in selectors.
  • Consider whether request headers should be stored alongside params in the collections store.
  • Consider how FETCH_ONE, CREATE, UPDATE, and DELETE may/may not need to persist response headers.

For future reference: given a Fetch API response object, you can convert the headers to a plain object like this:

Array.from(response.headers.entries()).reduce((obj, [key, value]) => {
  obj[key] = value
  return obj
}, {})
@devvmh
Copy link
Owner Author

devvmh commented Dec 12, 2017

See also #65 for discussion about passing a parser to the ApiClient.

Renaming this issue because it probably isn't the roadmap anymore

@devvmh devvmh changed the title Roadmap for 6.x possible breaking changes for 6.x Dec 12, 2017
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

No branches or pull requests

1 participant