Deprecate eager transformValue in favor of computeAttribute
Hello @hjdivad finally sat down to tackle what we talk about in #810, this is a proof of concept and would love your feedback, there's only a few failing tests but most if not all of them would be solved when we figure out the new api for the m3-schema-manager and the m3-schema
One of my main questions I guess:
Do we proceed to deprecate the schema.models all together? I think its not currently possible because Model depends indirectly on it via schema.isAttributeIncluded for example
https://github.com/hjdivad/ember-m3/blob/1a3390cc9bf3bc4ef2329179bab362dae51eb9fa/addon/model.js#L283
We could keep using the m3-schema-manager as the interface and delegate to the m3-schema service and have some basic examples to implement the same functionality, this would mean more boilerplate... but allows more flexibility
And/Or, we could basically move the m3-schema-manager schema.models details to the m3-schema so we can still support the schema.models and expect users to extend...
This PR also includes the schemaInterface.topModel() method, not sure if that's the best way to reach the topModel from a recordData
@betocantu93 awesome, thanks for taking this on. I'm very excited to see this evolution -- I think it will make the overall system more flexible and also have a simpler mental model.
Just a checkpoint, for now I removed the hardcoded m3-schema-manager schema.models lookups for optional hooks in the schema, I also created a schema-compat blueprint which implements these hooks to provide an optional "more featured schema" out of the box.
Questions/todos/thoughts I think I need more context
- IMHO I'm not entirely sure about removing
isAttributeIncludedas it would complicatecomputeAttributemental model because now you have to think about when its called or why it was called (bynotifyPropertyChange,unknownPropertyorsetUnknownProprety, the three places that currently use this hook), and also to returnschemaInterface.getAttr(key)(?) – I may need more context – but I like the mental model that computeAttribute is used to calculate values only, so I can transform them and then return one of this 4 types ember-m3 support, but that's it. And so, for now... I just left it working as before, hopefully I can get more feedback and implement it in a follow-up PR - I'm not sure how to patch the tests, because there's a few that assert on schema.models semantics, should I deprecate those? or somehow (not sure how) import the schema-compat blueprint and use it?
Also another after thought about isAttributeIncluded refactor... again I may need more context and this could be part of the implementation details but here
https://github.com/hjdivad/ember-m3/blob/23279ea70e1536074f599346896a01830a06cd33/addon/model.js#L525
Using the proposed computeAttribute returning undefined semantics to stop working or early return on that particular set, notify or get, would mean that the recordData have that data via _attributes, _inFlightAttributes or _data because If I understood correctly, computeAttribute would be expected to return something other than undefined via return schemaInterface.getAttr(key)?, which will not always be the case, specially to allow dynamic sets
I just refactored a bunch of our MegamorphicModel.reopen stuff for our app with this cleaner API. I also just renamed the blueprints, and so we could close #735 too if this gets merged