feat: Add helpers and parser by lebojo · Pull Request #67 · Infomaniak/design-system

lebojo · 2026-03-24T12:38:28Z

This PR adds Helpers and parser that Style dictionary needs to work

github-actions · 2026-03-24T12:39:14Z

✅ Storybook build successful

Decision: relevant files changed
Changed files inspected: 3
Workflow run: View details
Artifact: not uploaded (deploy succeeded)
Deployment: Open Storybook

Relevant files

packages/tokens/scripts/scripts/build-tokens/src/build/outputs/swift/helpers.ts
packages/tokens/scripts/scripts/build-tokens/src/build/outputs/swift/parser.ts
packages/tokens/scripts/scripts/build-tokens/src/build/outputs/swift/transforms.ts

github-actions · 2026-03-24T12:40:49Z

PR Reviewer Guide 🔍

(Review updated until commit `f71858c`)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Missing Error Handling The parser calls `JSON.parse(contents)` without a try-catch block. If a token file contains malformed JSON, this will throw an unhandled exception and crash the build process. Wrap the parsing logic in a try-catch to provide a meaningful error message indicating which file failed to parse. const data = JSON.parse(contents); Incorrect Array Handling The check `typeof val === 'object'` returns true for arrays. If a top-level property is an array (e.g., a metadata field), the code will incorrectly add a `$type` property to the array object. Add `!Array.isArray(val)` to the condition to ensure only plain objects receive the inherited type. if (val && typeof val === 'object' && !(val as any)['$type']) { (val as any)['$type'] = rootType;

github-actions · 2026-03-24T12:45:46Z

+    parser: ({ contents }: { filePath: string; contents: string }) => {
+        const data = JSON.parse(contents);


Suggestion: Wrap JSON.parse(contents) in a try-catch block to handle malformed JSON files gracefully. This prevents the build process from crashing with an unhandled exception and allows for a more descriptive error message that includes the filePath. [possible issue, importance: 7]

Suggested change

parser: ({ contents }: { filePath: string; contents: string }) => {

const data = JSON.parse(contents);

parser: ({ contents, filePath }: { filePath: string; contents: string }) => {

let data: any;

try {

data = JSON.parse(contents);

} catch (e) {

throw new Error(`Failed to parse ${filePath}: ${e instanceof Error ? e.message : String(e)}`);

}

github-actions · 2026-03-24T12:49:14Z

Persistent review updated to latest commit f71858c

github-actions · 2026-03-24T13:01:22Z

Failed to generate code suggestions for PR

vjo · 2026-03-25T13:30:51Z

+export const T1_DIRECTORY_NAME = 't1-primitive';
+export const T2_DIRECTORY_NAME = 't2-semantic';
+export const T3_DIRECTORY_NAME = 't3-component';


Nit: these constants are already defined in /packages/tokens/scripts/scripts/build-tokens/src/constants/design-token-tiers.ts, could we re-use them?

vjo · 2026-03-25T13:32:12Z

@@ -0,0 +1,31 @@
+import { AUTO_GENERATED_FILE_HEADER } from '../../constants/auto-generated-file-header.ts';
+
+export const VARIABLE_PREFIX = 'esds';


Nit: could we re-use or defined this constant in the same folder as CSS_VARIABLE_PREFIX, see packages/tokens/scripts/scripts/build-tokens/src/constants/css-variable-prefix.ts

vjo · 2026-03-25T13:34:48Z

+ * Name transform that replicates the exact naming logic from the custom DTCG framework.
+ */
+export const nameTransform: Transform = {
+  name: 'esds/name',


Question: should esds come from a constant?

I think you're right!

vjo · 2026-03-25T13:35:39Z

+import { segmentKebabCase } from './helpers.ts';
+
+/**
+ * Name transform that replicates the exact naming logic from the custom DTCG framework.


Question: how is DTCG custom? It should be a standard, no?

Do you mean "custom parser" ?

Not, it's not a parser, it's for the output.
But I think it's an oversight of my part with testing cause actually SD have a Kebabcase transform built-in

vjo · 2026-03-25T13:36:47Z

+ * source files define different $type at their root level.
+ */
+export const fixTypeInheritanceParser = {
+  name: 'esds/fix-type-inheritance',


Question: should esds come from a constant?

vjo · 2026-03-25T13:38:16Z

@@ -0,0 +1,23 @@
+/**
+ * Custom parser that moves root-level $type into each top-level group.
+ * This prevents SD5's merge from losing per-file $type when multiple


Question: what does SD5 mean here?

Style Dictionary 5

Suggested change

* This prevents SD5's merge from losing per-file $type when multiple

* This prevents Style Dictionary's merge from losing per-file $type when multiple

lebojo self-assigned this Mar 24, 2026

github-actions bot requested review from lifaon74 and vjo March 24, 2026 12:38

lebojo mentioned this pull request Mar 24, 2026

chore: Add Style dictionary depedency #68

Draft

3 tasks

github-actions bot added the Review effort 2/5 label Mar 24, 2026

lebojo mentioned this pull request Mar 24, 2026

feat: Style dictionary context models #69

Draft

github-actions bot reviewed Mar 24, 2026

View reviewed changes

feat: Add helpers and parser

f71858c

lebojo force-pushed the feat/ios/helpers-parser branch from 1cf15e3 to f71858c Compare March 24, 2026 12:46

vjo reviewed Mar 25, 2026

View reviewed changes

		parser: ({ contents }: { filePath: string; contents: string }) => {
		const data = JSON.parse(contents);

-    parser: ({ contents }: { filePath: string; contents: string }) => {
-        const data = JSON.parse(contents);
+    parser: ({ contents, filePath }: { filePath: string; contents: string }) => {
+        let data: any;
+        try {
+            data = JSON.parse(contents);
+        } catch (e) {
+            throw new Error(`Failed to parse ${filePath}: ${e instanceof Error ? e.message : String(e)}`);
+        }

		@@ -0,0 +1,31 @@
		import { AUTO_GENERATED_FILE_HEADER } from '../../constants/auto-generated-file-header.ts';

		export const VARIABLE_PREFIX = 'esds';

	* This prevents SD5's merge from losing per-file $type when multiple
	* This prevents Style Dictionary's merge from losing per-file $type when multiple

Conversation

lebojo commented Mar 24, 2026

Uh oh!

github-actions bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Storybook build successful

Relevant files

Uh oh!

github-actions bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit f71858c)

Uh oh!

github-actions bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 24, 2026

Uh oh!

github-actions bot commented Mar 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Mar 24, 2026 •

edited

Loading

github-actions bot commented Mar 24, 2026 •

edited

Loading

(Review updated until commit `f71858c`)