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

Avoid printing the entire file source code on error when watching #14605

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bogdan
Copy link

@bogdan bogdan commented Oct 5, 2024

Avoid printing the entire source file to console when watcher encounters an error.

Currently, the CssSyntaxError is highly unreadable when printed because the source file is printed entirely after the backtrace which makes it necessary to scroll up the console to see more useful information.
This patch only leaves the part that is usually useful: file name, line color, error message and backtrace:

Rebuilding...
CssSyntaxError: tailwindcss: /Users/bogdan/makabu/brad/seven-figures/app/assets/stylesheets/application.tailwind.css:9:5: The `qq` class does not exist. If `qq` is a custom class, make sure it is defined within a `@layer` directive.
    at Input.error (/Users/bogdan/makabu/brad/seven-figures/node_modules/postcss/lib/input.js:106:16)
    at AtRule.error (/Users/bogdan/makabu/brad/seven-figures/node_modules/postcss/lib/node.js:115:32)
    at processApply (/Users/bogdan/makabu/brad/seven-figures/node_modules/tailwindcss/lib/lib/expandApplyAtRules.js:380:29)
    at /Users/bogdan/makabu/brad/seven-figures/node_modules/tailwindcss/lib/lib/expandApplyAtRules.js:551:9
    at /Users/bogdan/makabu/brad/seven-figures/node_modules/tailwindcss/lib/processTailwindFeatures.js:55:50
    at async Object.Once (/Users/bogdan/makabu/brad/seven-figures/node_modules/tailwindcss/lib/cli/build/plugin.js:246:17)
    at async LazyResult.runAsync (/Users/bogdan/makabu/brad/seven-figures/node_modules/postcss/lib/lazy-result.js:261:11)
    at async Object.watch (/Users/bogdan/makabu/brad/seven-figures/node_modules/tailwindcss/lib/cli/build/plugin.js:371:13)
    at async build (/Users/bogdan/makabu/brad/seven-figures/node_modules/tailwindcss/lib/cli/build/index.js:47:9)
Example Before the patch
Rebuilding...
CssSyntaxError: tailwindcss: /Users/bogdan/makabu/brad/seven-figures/app/assets/stylesheets/application.tailwind.css:9:5: The `qq` class does not exist. If `qq` is a custom class, make sure it is defined within a `@layer` directive.
    at Input.error (/snapshot/tailwindcss/node_modules/postcss/lib/input.js:106:16)
    at AtRule.error (/snapshot/tailwindcss/node_modules/postcss/lib/node.js:115:32)
    at processApply (/snapshot/tailwindcss/lib/lib/expandApplyAtRules.js:380:29)
    at /snapshot/tailwindcss/lib/lib/expandApplyAtRules.js:551:9
    at /snapshot/tailwindcss/lib/processTailwindFeatures.js:55:50
    at async Object.Once (/snapshot/tailwindcss/lib/cli/build/plugin.js:245:17)
    at async LazyResult.runAsync (/snapshot/tailwindcss/node_modules/postcss/lib/lazy-result.js:261:11)
    at async Object.watch (/snapshot/tailwindcss/lib/cli/build/plugin.js:370:13)
    at async build (/snapshot/tailwindcss/lib/cli/build/index.js:47:9) {
  reason: 'The `qq` class does not exist. If `qq` is a custom class, make sure it is defined within a `@layer` directive.',
  file: '/Users/bogdan/makabu/brad/seven-figures/app/assets/stylesheets/application.tailwind.css',
  source: '@import "choices.css";\n' +
    '\n' +
    '@tailwind base;\n' +
    '@tailwind components;\n' +
    '@tailwind utilities;\n' +
    '\n' +
    '@layer components {\n' +
    '  .page-header {\n' +
    '    @apply text-4xl font-bold mb-4 text-center qq; \n' +
    '  }\n' +
    '  .card-standard {\n' +
    '    @apply bg-white shadow-xl dark:bg-gray-800;\n' +
    '  };\n' +
    '  .card-central {\n' +
    '    @apply mx-auto max-w-lg w-full;\n' +
    '  }\n' +
    '  .text-fade {\n' +
    '    @apply text-base-300;\n' +
    '  }\n' +
    '  .admin-filters-grid {\n' +
    '    /* @apply columns-1 sm:columns-2 lg:columns-3 2xl:columns-4 w-full break-inside-avoid */\n' +
    '    /* @apply grid grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 2xl:grid-cols-4 w-full */\n' +
    '  }\n' +
    '  .h1 {\n' +
    '    @apply text-4xl\n' +
    '  }\n' +
    '  .h2 {\n' +
    '    @apply text-3xl\n' +
    '  }\n' +
    '  .h3 {\n' +
    '    @apply text-2xl\n' +
    '  }\n' +
    '  .h4 {\n' +
    '    @apply text-xl\n' +
    '  }\n' +
    '  .h5 {\n' +
    '    @apply text-lg\n' +
    '  }\n' +
    '}\n' +
    '\n' +
    'input[type="number"]::-webkit-outer-spin-button, input[type="number"]::-webkit-inner-spin-button {\n' +
    '  -webkit-appearance: none;\n' +
    '  margin: 0;\n' +
    '}\n' +
    '\n' +
    'input[type="number"] {\n' +
    '  -moz-appearance: textfield;\n' +
    '}\n' +
    '.field-with-errors {\n' +
    '  display: contents;\n' +
    '}\n',
  line: 9,
  column: 5,
  endLine: 9,
  endColumn: 51,
  input: {
    column: 5,
    endColumn: 51,
    endLine: 9,
    line: 9,
    source: '@import "choices.css";\n' +
      '\n' +
      '@tailwind base;\n' +
      '@tailwind components;\n' +
      '@tailwind utilities;\n' +
      '\n' +
      '@layer components {\n' +
      '  .page-header {\n' +
      '    @apply text-4xl font-bold mb-4 text-center qq; \n' +
      '  }\n' +
      '  .card-standard {\n' +
      '    @apply bg-white shadow-xl dark:bg-gray-800;\n' +
      '  };\n' +
      '  .card-central {\n' +
      '    @apply mx-auto max-w-lg w-full;\n' +
      '  }\n' +
      '  .text-fade {\n' +
      '    @apply text-base-300;\n' +
      '  }\n' +
      '  .admin-filters-grid {\n' +
      '    /* @apply columns-1 sm:columns-2 lg:columns-3 2xl:columns-4 w-full break-inside-avoid */\n' +
      '    /* @apply grid grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 2xl:grid-cols-4 w-full */\n' +
      '  }\n' +
      '  .h1 {\n' +
      '    @apply text-4xl\n' +
      '  }\n' +
      '  .h2 {\n' +
      '    @apply text-3xl\n' +
      '  }\n' +
      '  .h3 {\n' +
      '    @apply text-2xl\n' +
      '  }\n' +
      '  .h4 {\n' +
      '    @apply text-xl\n' +
      '  }\n' +
      '  .h5 {\n' +
      '    @apply text-lg\n' +
      '  }\n' +
      '}\n' +
      '\n' +
      'input[type="number"]::-webkit-outer-spin-button, input[type="number"]::-webkit-inner-spin-button {\n' +
      '  -webkit-appearance: none;\n' +
      '  margin: 0;\n' +
      '}\n' +
      '\n' +
      'input[type="number"] {\n' +
      '  -moz-appearance: textfield;\n' +
      '}\n' +
      '.field-with-errors {\n' +
      '  display: contents;\n' +
      '}\n',
    url: 'file:///Users/bogdan/makabu/brad/seven-figures/app/assets/stylesheets/application.tailwind.css',
    file: '/Users/bogdan/makabu/brad/seven-figures/app/assets/stylesheets/application.tailwind.css'
  },
  plugin: 'tailwindcss'
}

src/cli/build/plugin.js Outdated Show resolved Hide resolved
@philipp-spiess
Copy link
Member

@bogdan I’m not sure this is a useful change. Most people wouldn't need the tailwind stack trace but instead find the error in their CSS file. In the example you posted, the full message also contains the valuable error message:

The qq class does not exist. If qq is a custom class, make sure it is defined within a @layer directive.',

Which is now hidden with your change.

@bogdan
Copy link
Author

bogdan commented Oct 11, 2024

The message is not hidden, you need to scroll right to see it. My point is that printing the entire source file to console when encountering an error is not a best practice, any library I saw before prints just file name and line number. As the error handling in this context is not specific (all errors handled the same way) , the best practice is to print the stack trace because we have no idea about error nature. The better approach would be to handle CSS syntax error differently than any other error eg print the error message only with no trace as it doesn't matter in this case. Do you want me to do that?

@thecrypticace thecrypticace requested a review from a team as a code owner December 3, 2024 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants