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

[Question] options argument type in Transform.transformer is not optional #975

Closed
m-yoshiro opened this issue May 15, 2023 · 2 comments
Closed
Labels

Comments

@m-yoshiro
Copy link

m-yoshiro commented May 15, 2023

Hello everyone,

I'm having a problem with [email protected] and Typescript.
When running tests in my project, this error occurred.

I think the problem came from #926, where Transform.transformer type was modified. options argument has been added and its type is not optional.

I'd like to ask if this behavior is intentional or not. Thanks!

    src/transformers/typographyToCss.test.ts:59:42 - error TS2554: Expected 2 arguments, but got 1.

    59     const cssFontValue = typographyToCss.transformer(mockToken);
                                                ~~~~~~~~~~~~~~~~~~~~~~

      node_modules/style-dictionary/types/Transform.d.ts:23:5
        23     options: Platform<PlatformType>
               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        An argument for 'options' was not provided.

p.s. I found that some transfomer's tests don't use options argument, so I suppose it might be unintentional.

var value = transforms["size/compose/remToSp"].transformer({
value: "1"
});
expect(value).toBe("16.00.sp");


In Addition, my typographyToCss 's implementation and test code are like below.

// Implementation
export const typographyToCss: Transform = {
  type: 'value',
  transitive: true,
  matcher: isTypography,
  transformer: (token) => {
    const { fontFamily, fontWeight, lineHeight, fontSize } = token.original
      .value as TypographyTokenValue;

    const output = `${fontWeight} ${fontSize}/${lineHeight} ${fontFamily}`;
    return output;
  },
};
// Test
const cssFontValue = typographyToCss.transformer(mockToken);

expect(cssFontValue).toBe(
  `${fontWeight} ${fontSize}/${lineHeight} ${fontFamily}`
);
@dbanksdesign
Copy link
Member

Thank you for the question @m-yoshiro! That change was intentional because when Style Dictionary calls that function, it always passes in the platform configuration object, so we want the custom transformer functions to not have to do an undefined check in their transformer function. This does make calling the transformer function in a unit test a bit more difficult, but you can just add an empty object as the 2nd argument in your unit test.

I'm open to suggestions on how to improve the typing of this though.

@m-yoshiro
Copy link
Author

Thank you for your explanations! I understand the reason and It's clear to me now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants