Skip to content

feat(tokens): use typescript in the tokens package #5557

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

Open
wants to merge 32 commits into
base: main
Choose a base branch
from

Conversation

myrta2302
Copy link
Contributor

@myrta2302 myrta2302 commented May 30, 2025

πŸ“„ Description

This PR implements Typescript in the Tokens Package.

πŸ“ Checklist

  • βœ… My code follows the style guidelines of this project
  • πŸ› οΈ I have performed a self-review of my own code
  • ⚠️ My changes generate no new warnings or errors
  • βœ”οΈ New and existing unit tests pass locally with my changes

@myrta2302 myrta2302 linked an issue May 30, 2025 that may be closed by this pull request
Copy link

changeset-bot bot commented May 30, 2025

⚠️ No Changeset found

Latest commit: 373f335

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@swisspost-bot
Copy link
Contributor

swisspost-bot commented Jun 27, 2025

Related Previews

@myrta2302 myrta2302 marked this pull request as ready for review June 30, 2025 06:46
@myrta2302 myrta2302 requested a review from a team as a code owner June 30, 2025 06:46
@myrta2302 myrta2302 requested a review from leagrdv June 30, 2025 06:46

if (meta.layer === 'semantic') {
const baseSetName = meta.setNames[0];
const overrideSetNameIndex = meta.setNames.findIndex(setName => setName === currentSetName);
const overrideSetNameIndex = meta.setNames.findIndex(
(setName: string) => setName === currentSetName,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, the : string would not be necessary if options are of the correct type.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the :string is still there

@myrta2302 myrta2302 requested a review from leagrdv July 18, 2025 09:55
@@ -48,7 +49,7 @@ StyleDictionary.registerFormat({
const utilityTokens = new Map();

dictionary.allTokens.forEach(token => {
const { subitem, state } = token.attributes;
const { subitem, state } = token.attributes || {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hides the typing error but it doesn't really fix it.
token.attributes is generally defined as Record<string, unknown> but we should specify it more to reflect what we get, which is something like this:

  attributes: {
    category: 'typo',
    type: 'post',
    item: 'utility',
    subitem: 'font-size',
    state: '11'
  },

],
});

config.platforms = Object.entries(config.platforms).reduce(
config.platforms = Object.entries(config.platforms || {}).reduce(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would hide a bug if config.platforms were to not be defined. It might be better to throw an error if config.platforms isn't defined instead. What do you think?

Something like this beforehand:
if (!config.platforms) return;


if (meta.layer === 'semantic') {
const baseSetName = meta.setNames[0];
const overrideSetNameIndex = meta.setNames.findIndex(setName => setName === currentSetName);
const overrideSetNameIndex = meta.setNames.findIndex(
(setName: string) => setName === currentSetName,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the :string is still there

@myrta2302 myrta2302 requested a review from leagrdv August 15, 2025 10:08
Copy link

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.

Use TypeScript in the tokens package
3 participants