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

feat(formats): add javascript/es6-ts-declarations formats for .d.ts #467

Closed
wants to merge 1 commit into from

Conversation

tychi-nflx
Copy link

@tychi-nflx tychi-nflx commented Sep 30, 2020

Possibly resolves #425

Description of changes:
Adds a format for exporting es6-ts-declarations

Usage (config.json):

...
  {
    "format": "javascript/es6-ts-declarations",
    "destination": "colors.d.ts"
  }
...

Output (colors.d.ts):

export const ColorBackgroundBase : string;
export const ColorBackgroundAlt : string;

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@chazzmoney
Copy link
Collaborator

Thank you for the contribution!

Two thoughts:

  1. Usually in the test, (where possible) we like to actually execute the library and attempt to import the styles. This could be doable in this case potentially, but I leave it to you. Alternatively, executing a test against the expected string literal for each of the styles listed could be a good alternative to the RegEx technique, especially regarding the next thought...
  2. Would all styles be of type 'string'? Maybe this makes sense, but it seems like maybe different style types might have different TS types. Maybe this is fine as is for a first step and can be changed later as usage develops.

@chazzmoney
Copy link
Collaborator

Hi @tychi-nflx,

We are looking at merging this but would like to hear your thoughts before we do.

Thanks!

@tychi-nflx
Copy link
Author

Hey @chazzmoney! Sorry for the delay, I ended up switching teams in the middle of the last month, so I'm not actually doing any further work on the design system I was helping out with.

Anything you decide, I'll be happy with :)

@ojassvi
Copy link

ojassvi commented Dec 15, 2020

Any updates on this when will this be merged to the base branch.

@calvinf
Copy link

calvinf commented Dec 15, 2020

Functionally, this looks good to me. Semantically, I'd recommend naming the format as "typescript/declarations" as it is specifically for the TypeScript language (ES is up to es2020 now and this really doesn't have anything in it that's specific to es6). Other then that though, it would be great to have this merged as it would simplify TS declaration generation and avoids the need for creating one-off custom format for my project (or others who need to do the same thing).

@chazzmoney
Copy link
Collaborator

@tychi-nflx can you change the format to "typescript/declarations"?

Thanks!

@chazzmoney
Copy link
Collaborator

Hi @tychi-nflx - if you have a quick second to just update the name to "typescript/declarations", then we can get it merged.

Thank you!

@dbanksdesign
Copy link
Member

Closing this in favor of #557 Thank you!

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

Successfully merging this pull request may close these issues.

Declaring TypeScript types for design tokens that use the ES6 module format
6 participants