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

Possible bug or need for documentation: Files run async #1341

Open
lukasoppermann opened this issue Sep 18, 2024 · 5 comments
Open

Possible bug or need for documentation: Files run async #1341

lukasoppermann opened this issue Sep 18, 2024 · 5 comments

Comments

@lukasoppermann
Copy link
Contributor

Hey,
I spent two days debugging my code because I got a racing condition with files.

It seems that from v3 to v4 there was a change and now the files defined in the files object run in parallel.

A common approach before this change was to write general fornats first and then overwrite the specific files afterwards, e.g.

    files: [
      {
        destination: `${outputFile}`,
        format: `css/advanced`,
        filter: token => isSource(token)
        options: {
          showFileHeader: false,
          outputReferences: false,
          descriptions: false,
          queries: getCssSelectors(outputFile),
          ...options?.options,
        },
      },
      {
        destination: `${outputFile}`,
        format: `css/customMedia`,
        filter: token => isSource(token) && token.$type === 'custom-viewportRange',
        options: {
          showFileHeader: false,
        },
      }
]

This does not work anymore as sometimes the second file is generated while the first one is still writing to the same file.

While I appreciate this may be "incorrect" usage, I would prefer the old way. If this is not desired, maybe an information that the files are not written syncronously but async would be helpful.

@jorenbroekema
Copy link
Collaborator

Hmm yeah I think I would classify this under incorrect usage. The API was not designed for the purpose of sequentially writing to the same file. I'd have to know a little bit more about your use case to understand why you would want multiple files to be sequentially overwritten, because to me it seems like it just completely un-does the work of the previous file, so then I'm tempted to say "the first one shouldn't be there if the second one always overwrites it anyways"

@lukasoppermann
Copy link
Contributor Author

Hey @jorenbroekema,

the use case is just simplicity. I can run all tokens through a default setup and filter down afterwards and overwrite the files that have special formats.

Now I have to add filters to every file like token is not of type ABC, etc.

If you feel it should be like this, maybe a "warning" could still be added, as the user does not know that the files are written async and v3 used to write in sync.

@jorenbroekema
Copy link
Collaborator

https://styledictionary.com/version-4/migration/#asynchronous-api we can add a caution thingie here to mention that most methods in SD that tend to loop over stuff (e.g. platforms & files) now run in parallel and if you were relying on them running in sequence, that would break.

This was a pretty huge performance improvement for many use cases so I don't want to revert that

@lukasoppermann
Copy link
Contributor Author

This was a pretty huge performance improvement for many use cases so I don't want to revert that

Totally get it and there are ways to work around it, e.g. adding more filtering to every step.
I just think it is important to mention.

Personally I feel like it would be better to mention it here: https://styledictionary.com/reference/config/#platform

E.g.

Files to be generated for this platform asynchronously.

Or somewhere here.

@Charlene-Bx
Copy link
Contributor

Running on the same issue. I have a design system divided into themes, each of which has breakpoints. Since the files have identical paths, I'm obliged (at least that's how it seemed to me after many tests) to generate them separately by file so as not to “overwrite” them. I then use an action to merge surrounding them with the desired media queries, all together. But now that's not synchronous, sometimes it starts by medium media queries, and sometimes by the large one, thus modifying the output order too.

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

No branches or pull requests

3 participants