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

Incorrect filenames in logging messages? #1300

Open
chris-dura opened this issue Aug 8, 2024 · 1 comment
Open

Incorrect filenames in logging messages? #1300

chris-dura opened this issue Aug 8, 2024 · 1 comment

Comments

@chris-dura
Copy link

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

  • I have a trivial custom format that uses getReferences
  • I have a platform that outputs two files, a file with all the tokens, and one that filters out some tokens
  • When I run a build, the logging messages seem accurate, the all.css builds without issue and a warning is displayed when building filtered.css
// 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
Copy link
Collaborator

jorenbroekema commented Aug 9, 2024

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).

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

2 participants