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

Incorrect filenames in logging messages? #1300

Open chris-dura opened 3 months ago

chris-dura commented 3 months ago

I am seeing some odd behavior in the log messages that was very confusing, and may have caused me to chase a red herring.


// my-format.mjs

import { getReferences, usesReferences } from "style-dictionary/utils";

const format = async ({ dictionary, platform, options, file }) => {
  const { allTokens, tokens, unfilteredTokens, unfilteredAllTokens } = dictionary;
  const { outputReferences, formatting, usesDtcg } = options;

  const formatProperty = (token) => {
    let to_return_token = "";

    if (usesReferences(token.original.$value)) {
      const refs = getReferences(token.original.$value, tokens, {unfilteredTokens, warnImmediately: false});
    }

    // Do some things to format the token...
    to_return_token += `${token.name}: ${token.$value}`;
    to_return_token += `\n`;

    // Return the formatted token
    return to_return_token;
  };

  return `
  ${allTokens.map(token => formatProperty(token)).join('\n')}
`;
}

export default {
  name: "my/format",
  format
};

// config.mjs

export default {
  log: {
    warnings: "error",
    verbosity: "verbose"
  },
  source: ["tokens.json"],
  platforms: {
    docs: {
      log: {
        warnings: "warn",
        verbosity: "verbose"
      },
      transformGroup: "css",
      files: [
        {
          format: "my/format",
          destination: "all.css",
        },
        {
          format: "my/format",
          destination: "filtered.css",
          filter: (token) => token.name !== "colors-red",
        },
      ],
    },
  },
};

Output:

$ node build-tokens.mjs

docs
- all.css
- filtered.css

docs
✔︎ all.css
⚠️ filtered.css
While building filtered.css, filtered out token references were found; output may be unexpected. Ignore this warning if intentional.
Here are the references that are used but not defined in the file:
    colors.red
This is caused when combining a filter and `outputReferences`.

Now, I've decided that I don't want ALL tokens in all.css, I actually just want to filter out some component tokens, so I apply a filter to that file as well...


// config.mjs

export default {
  log: {
    warnings: "error",
    verbosity: "verbose"
  },
  source: ["tokens.json"],
  platforms: {
    docs: {
      log: {
        warnings: "warn",
        verbosity: "verbose"
      },
      transformGroup: "css",
      files: [
        {
          format: "my/format",
          destination: "all.css",
          filter: (token) => token.name !== "my-foo",
        },
        {
          format: "my/format",
          destination: "filtered.css",
          filter: (token) => token.name !== "colors-red",
        },
      ],
    },
  },
};

However, now when I run a build, the logging message seems inaccurate...

$ node build-tokens.mjs

docs
- all.css
- filtered.css

docs
⚠️ all.css
While building all.css, filtered out token references were found; output may be unexpected. Ignore this warning if intentional.
Here are the references that are used but not defined in the file:
    colors.red
This is caused when combining a filter and `outputReferences`.
✔︎ filtered.css
  1. It says there's a filter reference error while building all.css, however, the filter I applied should NOT have filtered any tokens out
  2. It says the references that are missing are colors.red, however, those references should only be missing in the filtered.css, not the all.css
  3. Both all.css and filtered.css were built and output properly, I'm just concerned about the warnings being incorrect

... I'm migrating a VERY complex sd@v3 setup, so I really hope I'm not just missing something simple, but if the logging messages can potentially point me to the wrong file, that will make debugging the migration almost impossible for me :/

I'm wondering if there's an async issue, where the all.css file is still "building" at the point we throw the warning for filtered.css, and SD improperly associates the filtered reference warning with the all.css file?

jorenbroekema commented 3 months ago

Yeah this is probably due to v4 using async a lot more, I've had to tackle other issues before where logs were ordered awkwardly.

I am kinda weirded out by the fact that this doesn't seem to just be the "ordering", it seems like the filtered warnings are categorized for the wrong output run as well, which is arguably worse than just messing up the order.

If someone wants to investigate, feel free! Since it's not that urgent I don't see this making it to the top of my prio list anytime soon.

Relevant files:

It seems to me that we're not storing any of the errors/warnings in the groupMessages util in a way where we namespace/key them. For most errors/warnings we have to probably namespace them by platform name, for filtered out refs we also have to namespace them by ${format}+${destination} (note that destination is optional prop). Then we can assure that we're grabbing the relevant logs for the current platform or output.

Amount of work is medium I think, complexity fairly low (in case someone is looking to pick it up).