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

headers being a CP may be a poor choice #4496

Closed
stefanpenner opened this issue Aug 12, 2016 · 4 comments
Closed

headers being a CP may be a poor choice #4496

stefanpenner opened this issue Aug 12, 2016 · 4 comments
Labels
🏷️ doc This PR adds/improves/or fixes documentation

Comments

@stefanpenner
Copy link
Member

data/addon/adapters/rest.js

Lines 215 to 228 in c6d233b

[volatile](/api/classes/Ember.ComputedProperty.html#method_volatile)
function to set the property into a non-cached mode causing the headers to
be recomputed with every request.
```app/adapters/application.js
import DS from 'ember-data';
export default DS.RESTAdapter.extend({
headers: Ember.computed(function() {
return {
"API_KEY": Ember.get(document.cookie.match(/apiKey\=([^;]*)/), "1"),
"ANOTHER_HEADER": "Some header value"
};
}).volatile()

Volatile is strange, specifically in a world of templates it has a complex meaning and for this use-case a function instead of a CP would more then likely be what we want.

Other motivation is attempting roll back the concept of .volatile some the public API, as most of the time when it is required, it is the source of something else being funky.

cc @krisselden

@bmac
Copy link
Member

bmac commented Aug 12, 2016

The ds-improved-ajax feature flag (enabled in the current beta) adds a new headersForRequest method that will become the recommended way to handle this usecase. Users will be encouraged to override headersForRequest instead of declaring a .volatile computed property.

@stefanpenner
Copy link
Member Author

The ds-improved-ajax feature flag (enabled in the current beta) adds a new headersForRequest method that will become the recommended way to handle this usecase. Users will be encouraged to override headersForRequest instead of declaring a .volatile computed property.

Very awesome. Should we close this issue when:

  1. that is enabled by default
  2. the docs are updated to encourage the new flow

Or consider it solved, and close it now?

@bmac
Copy link
Member

bmac commented Aug 12, 2016

I'm in favor of closing this when the docs have been updated to encourage the new flow.

@stefanpenner stefanpenner reopened this Aug 12, 2016
@fivetanley fivetanley changed the title headers being a CP may be poor choose headers being a CP may be a poor choice Sep 12, 2016
@runspired
Copy link
Contributor

As we never landed DS-improved-Ajax and as headers is no longer a volatile property in going to close this.

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

No branches or pull requests

4 participants