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

Ember Data : 2.5.0-beta.1 WARNING: FEATURE["ds-pushpayload-return"] is set as enabled, but FEATURE flags are only available in canary builds. #4213

Closed
rusanu opened this issue Mar 6, 2016 · 19 comments

Comments

@rusanu
Copy link

rusanu commented Mar 6, 2016

The warning seems overactive. The feature flag is required in beta builds too, not only in canary.

@rwjblue
Copy link
Member

rwjblue commented Mar 6, 2016

Hmm, the feature should not be usable in beta builds unless it is enabled by default.

@rusanu
Copy link
Author

rusanu commented Mar 6, 2016

The functionality of ds-pushpayload-return is not present unless I enable the feature flag, in 2.5.0-beta1

@rwjblue
Copy link
Member

rwjblue commented Mar 6, 2016

Ya, understood, I'm talking about how it should be ;)

All of the entries in https://github.com/emberjs/data/blob/beta/config/features.json should have true or false values (not null).

/cc @bmac

@rwjblue
Copy link
Member

rwjblue commented Mar 6, 2016

Hmm, ember-data@2.5.0-beta.1 isn't tagged on github. Are you talking about Ember 2.5.0-beta.1?

Also, in general, this warning in Ember should only warn on feature flags that it knows about (not all items in EmberENV.FEATURES). I'll open an issue/PR in Ember to fix that...

@workmanw
Copy link

workmanw commented Mar 6, 2016

I added ds-pushpayload-return. This was my first PR that lives behind a feature flag and I tried to follow the existing patterns for this. If there is a change needed to resolve this, I'd be happy to make it. I just need a little direction.

@bmac
Copy link
Member

bmac commented Mar 6, 2016

@workmanw you did everything correctly. This is my mistake.

Due to some time constraints this week I didn't manage to release 2.5.0-beta.1 at the same time as the 2.4.0 release. As a result I didn't update the https://github.com/emberjs/data/blob/beta/config/features.json on the beta branch. I will do that now.

Like @rwjblue said it should not be possible to enable feature flags on the beta branch unless they are enabled by default.

@rusanu
Copy link
Author

rusanu commented Mar 6, 2016

According to the log, it is Ember-data 2.5.0-beta.1

vendor.js:17201 WARNING: FEATURE["ds-pushpayload-return"] is set as enabled, but FEATURE flags are only available in canary builds.
vendor.js:16950 DEBUG: -------------------------------
vendor.js:16950 DEBUG: Ember      : 2.4.1
vendor.js:16950 DEBUG: Ember Data : 2.5.0-beta.1
vendor.js:16950 DEBUG: jQuery     : 1.11.3
vendor.js:16950 DEBUG: -------------------------------

@rusanu
Copy link
Author

rusanu commented Mar 6, 2016

Looks like a storm in a papercup :)

@rusanu
Copy link
Author

rusanu commented Mar 6, 2016

@workmanw Thanks for the pushPayload return. Very useful!

@pangratz
Copy link
Member

pangratz commented Apr 4, 2016

This has been fixed upstream by open source hero @rwjblue in emberjs/ember.js#13239.

@denzo
Copy link

denzo commented Nov 23, 2016

I'm using v2.10.0-beta.3 and when I enable it in my environment file I still get the warning mentioned above:

WARNING: FEATURE["ds-pushpayload-return"] is set as enabled, but FEATURE flags are only available in canary builds.

More importantly, it doesn't return anything.

I noticed that it is set to false here https://github.com/emberjs/data/blob/v2.10.0-beta.3/config/features.json#L4

Is it not possible to enable it in v2.10.0-beta.3?

@sdhull
Copy link

sdhull commented Aug 15, 2018

We're on ember-data 3.2.1 -- whatever happened to the pushpayload-return feature? Is it available in mainline? Do I still need to be on canary to enable it?

@runspired
Copy link
Contributor

@sdhull it was never advanced, and I would recommend not using store.pushPayload in general.

Here's a simple effective way to accomplish what you want with better guarantees: https://gist.github.com/runspired/96618af26fb1c687a74eb30bf15e58b6/

@sdhull
Copy link

sdhull commented Aug 15, 2018

@runspired ok thanks, good to know. So should pushPayload be deprecated? The way I read the docs makes me feel like that's the method to use (anytime I read "convenience wrapper" I'm like I like convenience...)

@runspired
Copy link
Contributor

@sdhull it will be eventually. store.push is a much better API. We will be replacing both of them eventually with an async version of pushing json-api documents.

@workmanw
Copy link

I'm no longer using ember-data so I don't have a need for this, but it is baffling that the answer is a code snippet requiring the developer to lookup the model class, look up the serializer, manually normalize the payload with the correct serializer and then push it into the store just to get back a model.

This is something that should be in ember-data. Even if my attempt at pushPayload was not the right direction, asking the developer to take this on the application side is crazy, especially given that all of those APIs have under gone a lot of churn over the years.

@runspired
Copy link
Contributor

@workmanw the above APIs have been the most stable public APIs in ember-data since the very early days.

Composing ember-data functionally in this manner is something every dev should know.

Writing your own serializer is something every application should do.

Finally, there is an API. If all you want is a model.. use json-api and go straight to the store.push step.

So basically: what the gist is saying is

  • if you don’t have json-api, then take the serializer you built and understand, use it to get json-api, then push the json-api into the store to update state and receive back Records.

There is nothing ember-data could ever do to provide normalization for non json-api payloads, as there’s no possible way for us to know how to normalize them.

This was the big flaw of pushPayload, and it was made even worse by store.pushPayload not passing the modelName to serializer.pushPayload and by that method requiring data to be normalized to a special undocumented format different from all other serializer formats prior to being given to store.pushPayload at all. Normalizing just to normalize again, all in a hacky hope-this-works way when in reality you just need to transform from your format to Json-api.

@workmanw
Copy link

We'll have to agree to disagree then. I think it's a completely flawed notion to expect users to have to compose several different components this way just to get back a model(s) they just pushed into the store. What's more is that this isn't documented anywhere there I've seen.

This kind of thing is the frustration that I've heard from everyone I've talked to who's used ember-data. "Why are these things so hard?", "Why doesn't ember-data just do that for me.".

@runspired
Copy link
Contributor

I think it's a completely flawed notion to expect users to have to compose several different components this way just to get back a model(s) they just pushed into the store.
Why doesn't ember-data just do that for me

It does. You want records? then push json-api into the store. It is literally the only way to create records. The spec is what allows us to know how to interpret the payloads we receive. What you are saying is you want impossible magic in which you can push arbitrary unstructured json into the store and expect us to know what it means. This desire for impossible magic is what has led too many folks into using bad patterns and hackish workarounds only to make simple things hard in ember-data.

What's more is that this isn't documented anywhere there I've seen.

Each of these methods is documented. That we haven't done a good job with a cookbook showing what can be done with the APIs we provided is though a failure on our part and a big part of why folks think ember-data is hard or confusing when it doesn't need to be.

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

8 participants