amzn / style-dictionary

A build system for creating cross-platform styles.
https://styledictionary.com
Apache License 2.0
3.93k stars 557 forks source link

Breaking Change in 3.9.1: token.filepath now uses system path delemiter #1066

Closed Vampire2008 closed 10 months ago

Vampire2008 commented 11 months ago

In 3.9.1 token.filepath now uses system path delemiter (for Windows it '\'). It breaks some filter checks in our project. We have maked changes. But please do not release some breaking changes in patch versions.

pinguet62 commented 11 months ago

Hello 🖖 I have also a regression with 3.9.1 in Windows. I don't known if it's linked to "token.filepath".

ℹ️ The version 3.9.0 with glob@^10.3.10 fails -> probably the root cause?

jorenbroekema commented 11 months ago

https://github.com/amzn/style-dictionary/commit/4b0b2714f72ee33bfc992cbc460be871407de4e3 seems related to this commit.

https://github.com/isaacs/node-glob/blob/main/changelog.md#1010 there seems to be a posix option added to make paths "/"-delimited even on windows, we might need to apply that. @Vampire2008 could you confirm perhaps by adding posix: true to the options at https://github.com/amzn/style-dictionary/blob/main/lib/utils/combineJSON.js#L48 (e.g. monkeypatch this in your node_modules)?

Maybe contribute a PR with this fix :) if it's indeed the fix for this regression?

Apologies for the breaking change, it seems we don't have a test that verifies the filePath prop values on tokens

Vampire2008 commented 11 months ago

I tested adding posix: true. It returned previous behavior. I already adapted my code to new behavior.

jorenbroekema commented 11 months ago

Working on this now.

https://github.com/amzn/style-dictionary/actions/runs/7288756295/job/19861941453?pr=1070 here I upgraded to latest glob and verified with windows github actions workflow that we have some failing tests related to this bug report. So currently busy making sure everything works on Windows, as well as our tests

seanogdev commented 10 months ago

Could we get a release with this fix out?

jorenbroekema commented 10 months ago

Created a PR, just gotta wait a bit for someone in the Amazon team to merge & release it

jorenbroekema commented 10 months ago

Done, 3.9.2 released with fix