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

Filter support across the board #86

Closed
davixyz opened this issue Apr 30, 2018 · 9 comments
Closed

Filter support across the board #86

davixyz opened this issue Apr 30, 2018 · 9 comments
Assignees
Labels
Core Architecture This is an issue related to the core architecture of Style Dictionary

Comments

@davixyz
Copy link
Contributor

davixyz commented Apr 30, 2018

Summary

While doing some custom implementation where I need to filter very granularly certain properties for specific platform I noticed that the filtering system seems to be applied on some templates only.
In iOS Templates we have:

<% var props = _.filter(allProperties, this.filter); %>

Which basically takes whatever is coming from the configuration object as filter.

In Android, it's slightly different, color for instance:

var props = _.filter(allProperties, function(prop) {

It will only filter colors... but if we want to add a custom filter, we can't.

Proposals

  1. Standardize filters within templates/formats to use whatever is in config and extend a certain configuration.
Example in Android:

var props = _.filter(allProperties, function(prop) {

Changes
var defaultFilter = {
  attributes: {
    category: "color"
  }
};
var props = _.filter(allProperties, Object.extend({}, this.filter, defaultFilter));
  1. Add this filtering pre-processing to any transform; So we don't need to pre-filter at template/format level but the library does it automatically for everything.
@dbanksdesign
Copy link
Member

I like this. The filtering on templates/formats has been a little inconsistent, it would be great to standardize it.

Could you go into proposal 2 a bit more?

@davixyz
Copy link
Contributor Author

davixyz commented Apr 30, 2018

I need to dig deeper on the library to understand the moving parts, but in general it would be that we need to add the filters before the format/template output starts to happen.

I can see that we have some opportunity to do it somewhere here:

_.each(platform.files, function (file) {

Since at that time we have reference to each file and thus their own filters

Or maybe before running the format?

fs.writeFileSync(destination, format(dictionary, platform));

But one problem that I see is that in formatters or templates we access differently the properties, some use .allProperties

var root_vars = ':root {\n' + variablesWithPrefix(' --', dictionary.allProperties) + '\n}\n';

And others .properties

JSON.stringify(dictionary.properties, null, 2) + ';';

So that means we would have to modify the two objects before passing them down... or standardize that all formats/templates use the same properties.

Thoughts?

@dbanksdesign
Copy link
Member

dbanksdesign commented May 1, 2018

I see what you are saying now, trying to filter the style dictionary before it gets passed to the format or template. Definitely doable. It is a philosophical question on separation of concerns. Should it be the format/template's job to do the filtering, or the build system?

Pros for build system:

  • DRYer code (don't have to repeat the filtering in every format/template that ships with the framework)

Cons for build system:

  • Potentially one less vector of flexibility. If our idea of what filter does is not exactly what everyone wants, it could cause issues. If the filtering is part of templates/formats, users could just write their own.

On some templates/formats using .properties and others using .allProperties there is some discussion on this issue: #58 with some ideas how to clean that up a bit.

@davixyz
Copy link
Contributor Author

davixyz commented May 1, 2018

I see what you mean; I think philosophical you are right, the build system should not prevent getting any value to the formats or to the templates, it should be just a pass through map to a certain function and the formats/templates should define what they want and how they want it... at that time we can specify the filters. It's a little bit of repetition to create the same function everywhere but it's more flexible since it's going to be less ambigious, formats/templates just get all the data, do what you want with them.

Let me issue a PR about this and let me know

@chazzmoney
Copy link
Collaborator

Maybe this is a good time to rethink and structure how style dictionary works?

I feel like we have these same questions in several areas - areas as stupid as pushing colored console output and areas as important as this item and formats.

Nothing wrong with one off fixes, but with the 2.2 milestone intention on the horizon it might be good to sit down and hash out a system that functions similarly across the board?

@chazzmoney chazzmoney added the Core Architecture This is an issue related to the core architecture of Style Dictionary label Jun 6, 2018
@elliotdickison elliotdickison self-assigned this Jun 29, 2018
elliotdickison pushed a commit to elliotdickison/style-dictionary that referenced this issue Jun 29, 2018
These changes introduce a new filter API that addresses amzn#86. The key
differences are:
* Filters are applied automatically in the `buildFile` function to
  both the `properties` and `allProperties` members of a dictionary.
  Filters are no longer applied ad-hoc in formatters.
* A filter can be a function in addition to an object. If a function
  is provided the property is passed wholesale to the function and
  the result (true or false) is used to determine whether the property
  should be included or filtered out. If an object is provided the
  behavior is the same as it used to be (a partial deep comparison
  is performed using lodash).
elliotdickison pushed a commit to elliotdickison/style-dictionary that referenced this issue Jun 29, 2018
These changes introduce a new filter API that addresses amzn#86. The key
differences are:
* Filters are applied automatically in the `buildFile` function to
  both the `properties` and `allProperties` members of a dictionary.
  Filters are no longer applied ad-hoc in formatters.
* A filter can be a function in addition to an object. If a function
  is provided the property is passed wholesale to the function and
  the result (true or false) is used to determine whether the property
  should be included or filtered out. If an object is provided the
  behavior is the same as it used to be (a partial deep comparison
  is performed using lodash).
elliotdickison pushed a commit to elliotdickison/style-dictionary that referenced this issue Jun 29, 2018
These changes introduce a new filter API that addresses amzn#86. The key
differences are:
* Filters are applied automatically in the `buildFile` function to
  both the `properties` and `allProperties` members of a dictionary.
  Filters are no longer applied ad-hoc in formatters.
* A filter can be a function in addition to an object. If a function
  is provided the property is passed wholesale to the function and
  the result (true or false) is used to determine whether the property
  should be included or filtered out. If an object is provided the
  behavior is the same as it used to be (a partial deep comparison
  is performed using lodash).
elliotdickison pushed a commit to elliotdickison/style-dictionary that referenced this issue Jun 29, 2018
These changes introduce a new filter API that addresses amzn#86. The key
differences are:
* Filters are applied automatically in the `buildFile` function to
  both the `properties` and `allProperties` members of a dictionary.
  Filters are no longer applied ad-hoc in formatters.
* A filter can be a function in addition to an object. If a function
  is provided the property is passed wholesale to the function and
  the result (true or false) is used to determine whether the property
  should be included or filtered out. If an object is provided the
  behavior is the same as it used to be (a partial deep comparison
  is performed using lodash).
@elliotdickison
Copy link

elliotdickison commented Jun 29, 2018

One note on my PR: I didn't touch the custom filters in the templates. The new API allows users to specify filters in their config files without format/template authors having to implement anything, but if format/template authors wish to they can still apply their own filters as well (since they can run any JS they want). I think this strikes a nice balance of having an simple, scalable API built into style-dictionary while still giving users flexibility via registering their own formats/templates.

elliotdickison pushed a commit to elliotdickison/style-dictionary that referenced this issue Jun 29, 2018
These changes introduce a new filter API that addresses amzn#86. The key
differences are:
* Filters are applied automatically in the `buildFile` function to
  both the `properties` and `allProperties` members of a dictionary.
  Filters are no longer applied ad-hoc in formatters.
* A filter can be a function in addition to an object. If a function
  is provided the property is passed wholesale to the function and
  the result (true or false) is used to determine whether the property
  should be included or filtered out. If an object is provided the
  behavior is the same as it used to be (a partial deep comparison
  is performed using lodash).
elliotdickison pushed a commit to elliotdickison/style-dictionary that referenced this issue Jul 4, 2018
These changes introduce a new filter API that addresses amzn#86. The key
differences are:
* Filters are applied automatically in the `buildFile` function to
  both the `properties` and `allProperties` members of a dictionary.
  Filters are no longer applied ad-hoc in formatters.
* A filter can be a function in addition to an object. If a function
  is provided the property is passed wholesale to the function and
  the result (true or false) is used to determine whether the property
  should be included or filtered out. If an object is provided the
  behavior is the same as it used to be (a partial deep comparison
  is performed using lodash).
dbanksdesign pushed a commit that referenced this issue Jul 26, 2018
These changes introduce a new filter API that addresses #86. The key
differences are:
* Filters are applied automatically in the `buildFile` function to
  both the `properties` and `allProperties` members of a dictionary.
  Filters are no longer applied ad-hoc in formatters.
* A filter can be a function in addition to an object. If a function
  is provided the property is passed wholesale to the function and
  the result (true or false) is used to determine whether the property
  should be included or filtered out. If an object is provided the
  behavior is the same as it used to be (a partial deep comparison
  is performed using lodash).
@dbanksdesign
Copy link
Member

Filters are now in there! (have not released to npm yet though)

@shaunbent
Copy link

Hey @dbanksdesign, when you expect this to be released. This feature will massively help me reduce some complexity in my implementation of style dictionary.

dbanksdesign pushed a commit that referenced this issue Aug 6, 2018
These changes introduce a new filter API that addresses #86. The key
differences are:
* Filters are applied automatically in the `buildFile` function to
  both the `properties` and `allProperties` members of a dictionary.
  Filters are no longer applied ad-hoc in formatters.
* A filter can be a function in addition to an object. If a function
  is provided the property is passed wholesale to the function and
  the result (true or false) is used to determine whether the property
  should be included or filtered out. If an object is provided the
  behavior is the same as it used to be (a partial deep comparison
  is performed using lodash).
@dbanksdesign
Copy link
Member

It is now live and available in npm in v2.3.0!

chazzmoney pushed a commit that referenced this issue Aug 13, 2018
These changes introduce a new filter API that addresses #86. The key
differences are:
* Filters are applied automatically in the `buildFile` function to
  both the `properties` and `allProperties` members of a dictionary.
  Filters are no longer applied ad-hoc in formatters.
* A filter can be a function in addition to an object. If a function
  is provided the property is passed wholesale to the function and
  the result (true or false) is used to determine whether the property
  should be included or filtered out. If an object is provided the
  behavior is the same as it used to be (a partial deep comparison
  is performed using lodash).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Architecture This is an issue related to the core architecture of Style Dictionary
Projects
None yet
Development

No branches or pull requests

5 participants