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

Better warnings #1194

Closed
Rich-Harris opened this issue Dec 29, 2016 · 6 comments
Closed

Better warnings #1194

Rich-Harris opened this issue Dec 29, 2016 · 6 comments

Comments

@Rich-Harris
Copy link
Contributor

Warnings are a bit hard to do things with at the moment, because they're just strings, meaning that if you want to (say) suppress all warnings about this being undefined, you have to do some regex hackery.

It would be better if the warning was an object similar to an error object, with a warning code, a message, and other applicable information like file, line and column (which would allow the CLI to print a code frame – see also #296).

To minimise breakage, the warning object should have a toString method that returns the message.

@Rich-Harris
Copy link
Contributor Author

Further thought: we could probably group certain warnings together. For example, all the implicit external dependency or 'this is undefined' warnings could be delivered in one go, with all the relevant details. Having a unified way to handle grouped warnings would be better than doing all the necessary book-keeping in an ad-hoc fashion.

@yairEO
Copy link

yairEO commented Jan 5, 2017

I want to see warning, but recently you've added feature which prints to the console simple linting errors. I do not want to see those, they are huge and I already have elegant eslint messages.

if i'll set the onwarn setting to an empty function, I might miss "real" important errors. what should I do?

@Rich-Harris
Copy link
Contributor Author

The new system is a lot more flexible in terms of how warnings are printed. By default it'll show a code frame etc, but because the warning is now an object rather than a string you can do this sort of thing:

onwarn ( warning ) {
  // simple handler
  console.warn( warning.message );
}

onwarn ({ loc, message }) {
  // print location if applicable
  if ( loc ) {
    console.warn( `${loc.file} (${loc.line}:${loc.column}) ${message}` );
  } else {
    console.warn( message );
  }
}

onwarn ( warning ) {
  // skip certain warnings
  if ( warning.code === 'UNUSED_EXTERNAL_IMPORT' ) return;

  // throw on others
  if ( warning.code === 'NON_EXISTENT_EXPORT' ) throw new Error( warning.message );

  // console.warn everything else
  console.warn( warning.message );
}

@yairEO
Copy link

yairEO commented Jan 5, 2017

Thank you very much for the fine explanation! would be nice to copy it to the WIKI

@Rich-Harris
Copy link
Contributor Author

@Finesse
Copy link

Finesse commented Oct 14, 2018

What about adding a helper function which renders a warning beautifully?

I.e. incapsulate this code

function printRollupWarning({loc, message, frame}, printer) {
  if (loc) {
    printer(`${loc.file} (${loc.line}:${loc.column}) ${message}`);
    if (frame) {
      printer(frame);
    }
  } else {
    printer(message);
  }
}

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