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.

Sign up for GitHub

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

Jump to bottom

javascript/es6 format doesn't handle newlines in comments #1420

Open
Zkrgu opened this issue Dec 19, 2024 · 2 comments · May be fixed by #1439
Open

javascript/es6 format doesn't handle newlines in comments #1420

Zkrgu opened this issue Dec 19, 2024 · 2 comments · May be fixed by #1439
Labels
4.0 enhancement good first issue help wanted

Comments

@Zkrgu
Copy link

Zkrgu commented Dec 19, 2024

Comments are inserted as a line comment here:

style-dictionary/lib/common/formats.js

Line 643 in 737a02b

return comment ? to_ret.concat(`// ${comment}`) : to_ret;

This has previously been fixed for some formats in #953

Example of failing token

{
    "token": {
        "value": 0,
        "comment": "This\nis not valid"
    },
    "another": {
        "value": 1,
    }
}

This fails to build with

SyntaxError: Missing semicolon. (6:3)
  4 |
  5 | export const token = 0; // This
> 6 | is not valid
  7 | export const another = 0;
  8 |
@jorenbroekema
Copy link
Collaborator

jorenbroekema commented Dec 19, 2024 •
edited
Loading

I'm a bit conflicted about adding the enhancement flag, it's a missing feature imo but it could also be seen as a bug I suppose.

Anyways, the createPropertyFormatter format helper utility has a fix for this I introduced in v3 for the CSS / SCSS / LESS / Stylus formats, see the addComment helper. We can probably make that reusable in some way, and reuse it for our JS formats since they use a similar syntax for comments (// and /* */). Actually, I'm not sure // is valid in CSS, but at least the latter one they do share

PRs welcome

@curious-companion
Copy link

My approach will sanitize the comments by:

  1. Escaping any occurrences of */ to prevent premature block closure using .replace(/\*\//g, '*\\/').
  2. Replacing newline characters (\n) with spaces to ensure the comment remains on a single line using .replace(/\n/g, ' ').
    Please tell me if there is issue with this approach!

@curious-companion curious-companion mentioned this issue Jan 18, 2025
Open
@Zkrgu Zkrgu linked a pull request Jan 24, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
4.0 enhancement good first issue help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: account for multiline comments in js/es6 format Zkrgu/style-dictionary
3 participants
@Zkrgu @jorenbroekema @curious-companion