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

RFC: Pass through attribute meta data to related Transform #1

Conversation

knownasilya
Copy link
Contributor

Would love comments! cc @igorT, @fivetanley

(RFC content pasted in by @machty)

  • Start Date: 2014-08-14
  • RFC PR: (leave this empty)
  • Ember Issue: (leave this empty)

Summary

For Ember Data. Pass through attribute meta data, which includes parentType, options, name, etc.,
to the transform associated with that attribute. This will allow provide the following function signiture updates to DS.Transform:

  • transform.serialize(deserialized, attributeMeta)
  • transform.deserialize(serialized, attributeMeta)

Motivation

The main use case is to be able to configure the transform
on a per-model basis making more DRY code. So the transform can be aware of type and options on DS.attr can
be useful to configure the transform for DRY use.

Detailed design

Implementing

The change will most likely start in eachTransformedAttribute, which gets the attributes for that instance via get(this, 'attributes'). In the forEach the name will be used to get the specific attribute, e.g.

var attributeMeta = attributes.get(name);
callback.call(binding, name, type, attributeMeta);

The next change will be in applyTransforms, where the attributeMeta parameter is added and passed to transform.deserialize as the second argument.

You also have to handle the serialization part in serializeAttribute, where you pass through the attribute parameter to transform.serialize.

Using

A convoluted example:

// Example based on https://github.com/chjj/marked library
App.PostModel = DS.Model.extend({
  title: DS.attr('string'),
  markdown: DS.attr('markdown', {
    markdown: {
      gfm: false,
      sanitize: true
    }
  })
});

App.TechnicalPostModel = DS.Model.extend({
  title: DS.attr('string'),
  gistUrl: DS.attr('string'),
  markdown: DS.attr('markdown', {
    markdown: {
      gfm: true,
      tables: true,
      sanitize: false
    }
  })
});

App.MarkdownTransform = DS.Transform.extend({
  serialize: function (deserialized, attributeMeta) {
    return deserialized.raw;
  },

  deserialize: function (serialized, attributeMeta) {
    var options = attributeMeta.options.markdown || {};

    return marked(serialized, options);
  }
});

Drawbacks

Extra API surface area, although not much. This could also potentially introduce tight coupling between models and transforms if used improperly, e.g. not returning a default value if using type checking.

Alternatives

  1. Passing the information from the server, which is a poor solution.
  2. Writing a new transform for each model/attribute that needs a variation. Although this might be a good solution sometimes if you extend a base transform.

Unresolved questions

Does the whole meta object need to be passed, or do we selectively pass in only the useful properties? Like
options and parentType and name..

@trek trek changed the title Pass through attribute meta data to related Transform RFC: Pass through attribute meta data to related Transform Aug 22, 2014
@annismckenzie
Copy link

+1

@fivetanley
Copy link
Member

I like this a lot and seems good. @igorT thoughts?

@workmanw
Copy link
Contributor

workmanw commented Jan 6, 2016

👍

@fivetanley
Copy link
Member

This was approved for implementation at the core team meeting. I'll be the champion, but if someone wants to be the implementer let me know. You might get to it faster. :)

@knownasilya
Copy link
Contributor Author

First RFC and over a year later! There is always hope! 👍

@knownasilya
Copy link
Contributor Author

@fivetanley should this be merged in? Or is that after implementation?

@fivetanley
Copy link
Member

Let's wait til after implementation. Seems easier to track.

@pangratz
Copy link
Member

Added via 85ef99c

@pangratz pangratz closed this Jan 27, 2016
chancancode pushed a commit that referenced this pull request Oct 27, 2017
chancancode pushed a commit that referenced this pull request Oct 27, 2017
Add drain, and link to WIP implementations.
rwjblue pushed a commit that referenced this pull request Jun 22, 2018
@knownasilya knownasilya deleted the transform-attribute-meta-parameter branch June 28, 2018 15:18
@jenweber jenweber mentioned this pull request Aug 13, 2018
kategengler referenced this pull request in kategengler/rfcs-1 Dec 21, 2018
runspired added a commit that referenced this pull request Jan 16, 2019
Adding proper @ember-data package name where missing
rwjblue pushed a commit that referenced this pull request Apr 5, 2019
@NullVoxPopuli NullVoxPopuli mentioned this pull request Mar 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Implementer T-ember-data RFCs that impact the ember-data library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants