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

Add fine-grained control for deprecation warnings #1672

Closed
xanf opened this issue Aug 29, 2020 · 2 comments
Closed

Add fine-grained control for deprecation warnings #1672

xanf opened this issue Aug 29, 2020 · 2 comments

Comments

@xanf
Copy link
Contributor

xanf commented Aug 29, 2020

Feature Description

Provide a way for fine-grained control of deprecation warnings, allowing each deprecation to be "silenced" one-by-one
Make config.showDeprecationWarnings to accept not only boolean, but also a fine-grained config, for example:

config.showDeprecationWarnings = [
  'find',
  'findAll',
  'isVisible',
  'options.attachToDocument',
  'options.methods'
];

Alternate approaches:

  • new config silenceDeprecationWarnings with reverse behavior. The benefit of silenceDeprecationWarnings is that if new deprecation will be added - it will compain about it while showDeprecationWarnings will silently swallow it
  • allowing to pass custom warnDeprecated function, so warnings could be handled in other way

Problem

When you have huge codebase it is a real pain to deal with all deprecations immediately. On the other side showDeprecationWarnings silences all warnings and will not complain if people are using deprecated function, which was already "cleaned" from the codebase

Expected behavior

VTU should show deprecation warnings for elements, specified in showDeprecationWarnings but keep silence for other deprecations

Alternatives

Right now I have such ugly code in setting my tests in Jest:

const originalConsoleError = global.console.error;
const ALLOWED_DEPRECATION_MESSAGES = [
  ...[
    // subject for automated codemod
    '`find`',
    '`findAll`',
    // requires custom matcher
    'isVisible',
    'setMethods',
    // 'is',
  ].map(method => `${method} is deprecated and will be removed in the next major version.`),
  'options.attachToDocument is deprecated in favor of options.attachTo', // +
  'overwriting methods via the `methods` property is deprecated',
  'name is deprecated and will be removed in the next major version', // easy
];

global.console.error = function consoleErrorWithSuppression(message, ...args) {
  if (
    message.startsWith('[vue-test-utils]') &&
    ALLOWED_DEPRECATION_MESSAGES.find(warning => message.includes(warning))
  ) {
    console.warn(message, ...args);
  }

  originalConsoleError.call(console, message, ...args);
};

I'm quite uncomfortable with this hack and will be happy to submit PR as soon as the desired direction will be selected

@lmiller1990
Copy link
Member

lmiller1990 commented Aug 30, 2020

I don't think the hack is that bad - essentially this request is just to move that logic into VTU. I don't see a significant downside to adding this to VTU - it's purely additive. I wonder if many other people are having the problem (GL has likely one of the largest Vue code-bases on the planet, haha).

I don't have a strong opinion, either way. How about we get a fix for #1532 merged up then tackle this? I think #1532 has higher immediate value, and probably reduce your deprecation warnings a lot as well.

@xanf
Copy link
Contributor Author

xanf commented Aug 30, 2020

@lmiller1990 sure thing, I'll start my work on #1532 immediately, thanks for swift response

xanf added a commit to xanf/vue-test-utils that referenced this issue Sep 28, 2020
Allow passing custom handler for deprecation warnings allowing
fine-grained control how these are reported to a user
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