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

Add Custom Parser Support #85

Closed
dbanksdesign opened this issue Apr 25, 2018 · 10 comments
Closed

Add Custom Parser Support #85

dbanksdesign opened this issue Apr 25, 2018 · 10 comments
Labels
Core Architecture This is an issue related to the core architecture of Style Dictionary enhancement help wanted

Comments

@dbanksdesign
Copy link
Member

Acceptance Criteria

User should be able to supply a custom parser to the style dictionary to be used when reading property files. It should be done in a generic way so that any parser (yaml, hjson, and any other) could conceivably be used. A parser should be a function that takes a file (as a path string) as input, and outputs a JS object. Any function that follows this could be used as a parser.

How does this benefit the user?

Some do not like writing JSON, it can be verbose and not as simple as something like yaml. This will allow users to use any data formatting syntax to describe their style dictionaries. This fits in with the spirit of style dictionary in being flexible to accommodate any project/platform/framework. At its core, a style dictionary is a data structure for defining styles; the filetype the data structure is written in is irrelevant.

@jreichenberg
Copy link
Contributor

jreichenberg commented May 17, 2018

Easy half-step: allow a property file to be a node module that exports a js obj... utils/combineJSON could simply replace JSON.parse() with require(files[i]). If it's actually a json file require will parse it for you. Not the complete solution but non-invasive...

@chazzmoney chazzmoney added the Core Architecture This is an issue related to the core architecture of Style Dictionary label Jun 6, 2018
@elliotdickison
Copy link

I might give this a shot. My initial take is that a registerParser API would be consistent with the other extension APIs and would allow the user to specify multiple parser functions. The registerParser API would take both a parser function and the file extensions to which it applies. This would enable you to house properties in multiple file types. Is that over-engineering it? A couple use-cases I can think of:

  • users who want to keep most properties in static files (e.g. yaml), but need to have a few properties in dynamic files (e.g. JS) - or vice versa
  • users who are doing a slow migration from one file type to another (this might dovetail with some of the plans for an import system)

If we go forward with this I would extract the current JS/JSON parser into a parser function (basically just path => require(path)) and pre-apply it using the registerParser API (like what's being done with built-in transforms/formats). Thoughts?

@elliotdickison
Copy link

elliotdickison commented Jun 30, 2018

Just dug into the code a bit more. One important change that my proposed registerParser API would require is that the property files not be parsed immediately when the user calls extend (which they could potentially do before registering the necessary parsers). We'd have to wait until the user attempts to build the dictionary before parsing. I think intuitively that makes sense, I normally wouldn't expect applying configuration to actually run anything. This change would also help avoid the current edge-case where things might not work as expected if the user tries to pre-process or programmatically set up their property files (e.g. copy them from another dir) after they instantiate a dictionary but before they build it (something I actually ran into with a project).

@chazzmoney
Copy link
Collaborator

We should decide if we want to do this, or if we want to do something more direct like #19. Either way, probably target for a 3.0 release?

@elliotdickison
Copy link

@chazzmoney I don't think #19 is necessarily mutually exclusive with this, we could add the ability for custom parsers and then include those parsers out of the box. Thoughts?

Agreed about moving to 3.0. If there's still enough value in this I might have some time in a couple weeks to bang it out.

Do you have any thoughts on delaying the file parsing that I mentioned in my last comment?

@dbanksdesign
Copy link
Member Author

I agree about delaying parsing until we call exportPlatform (which is called internally from buildPlatform), but yea we should do that in the next major release to not break anything.

@chazzmoney
Copy link
Collaborator

Agreed on the delaying of parsing. 👍🏽

@chrisvxd
Copy link

chrisvxd commented Feb 4, 2020

Thought I'd share my custom build script to support YAML until it's officially supported: https://gist.github.com/chrisvxd/81137cb29b20a38c438c7918c776af4c

@dbanksdesign
Copy link
Member Author

This will be in the next major release: 3.0. The code is already merged into the 3.0 branch (#429)! 🚀

@dbanksdesign
Copy link
Member Author

This is already in the RC of 3.0 which you can get today with npm install style-dictionary@next

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 enhancement help wanted
Projects
None yet
Development

No branches or pull requests

5 participants