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

Fix deprecation warning #8622

Closed
jdhuntington opened this issue Apr 5, 2019 · 5 comments
Closed

Fix deprecation warning #8622

jdhuntington opened this issue Apr 5, 2019 · 5 comments
Assignees

Comments

@jdhuntington
Copy link
Contributor

(node:155424) [DEP0097] DeprecationWarning: Using a domain property in MakeCallback is deprecated. Use the async_context variant of MakeCallback or the AsyncResource class instead.
@ecraig12345
Copy link
Member

I tried to look into this and couldn't track down where it was coming from. Unless you can figure it out, I wonder if adding an "ignorable warnings" option to Rush would be the most practical path forward?

@octogonz
Copy link

Does anyone have some simple steps for reproing this warning?

@KevinTCoughlin
Copy link
Member

KevinTCoughlin commented Apr 17, 2019

@ecraig12345, did you try invoking common/scripts/install-run-rush.js with Node's --trace-warnings?

Reading nodejs/node#9516 (comment), the flag should output the stack trace. I haven't seen this warning myself with rush install & rush build on node v10.15.1 against branch fabric-7.

Edit: Nevermind, I got the warning with rush build on fabric-7 using node v10.15.1:

29 of 37: [@uifabric/fabric-website-resources] completed successfully in 4 minutes 26.8 seconds
[ssr-tests] started
[a11y-tests] started
30 of 37: [a11y-tests] completed successfully in 22.36 seconds
(node:6220) [DEP0097] DeprecationWarning: Using a domain property in MakeCallback is deprecated. Use the async_context variant of MakeCallback or the AsyncResource class instead.
31 of 37: [@uifabric/experiments] completed successfully in 3 minutes 49.0 seconds

Edit 2: I tried utilizing that flag in a few builds I suspected to be the culprit without luck. I tried the following packages: variants, merge-styles, and fluent-theme.

@octogonz
Copy link

octogonz commented Apr 17, 2019

I tried rush rebuild --verbose --parallelism 1 on that branch. The first warning occurred during webpack for @uifabric/variants:

[2:09:00 PM] ■ Running Webpack
[2:09:00 PM] ■ Webpack Config Path: D:\GitRepos\office-ui-fabric-react\packages\variants\webpack.config.js
[2:09:00 PM] ■ verify-api-extractor task is disabled in package.json
[2:09:00 PM] ■ finished 'verify-api-extractor' in 0s
Webpack version: 4.29.5
(node:13568) [DEP0097] DeprecationWarning: Using a domain property in MakeCallback is deprecated. Use the async_context variant of MakeCallback or the AsyncResource class instead.
[2:09:08 PM] ■ finished 'webpack' in 7.07s
[2:09:08 PM] ■ finished 'build' in 15.01s
22 of 37: [@uifabric/variants] completed with warnings in 17.37 seconds
[@uifabric/fluent-theme] started
>>> @uifabric/fluent-theme

It didn't repro the second time I tried to build this project, even if I did rushx clean. The reason is that terser-webpack-plugin creates a folder under variants/node_modules/.cache. (This is a misguided design, since secret hidden caches make the build nondeterministic, which is not good. Our toolchain should ideally find a way to move this into the standard variants/temp folder.) By manually deleting the .cache folder, the warning can be reproduced reliably.

The warning is reported directly from the node ../../scripts/just.js build NodeJS process. Someone had suggested this warning was caused by a native NPM package. I'm not sure about that. The only native DLL being loaded is office-ui-fabric-react\common\temp\node_modules\.registry.npmjs.org\node-sass\4.11.0\node_modules\node-sass\vendor\win32-x64-64\binding.node. Also, the NodeJS unit test for this warning is coded in pure JavaScript.

By adding process.emitWarning = function() { debugger; } to the start of just.js, I was able to catch the warning in the debugger. However, it is in a native callback, so there isn't much of a callstack (even with --trace-warnings).

However @kenotron I can see that just-task is using undertaker, which uses async-done 1.3.1, which creates a domain on this line. Maybe the Webpack task is invoking this callback from a different domain. (?)

@octogonz
Copy link

It occurred to me that, since this problem is limited to just-task, a simpleminded workaround would be to add something this to the top of just.js:

const originalEmitWarning = process.emitWarning;
process.emitWarning = function(warning, type, code, ctor) {
  if (code === 'DEP0097') {
    // Undertaker uses a deprecated approach that causes NodeJS 10 to print
    // this warning to stderr:
    //
    // "Using a domain property in MakeCallback is deprecated. Use the  async_context
    // variant of MakeCallback or the AsyncResource class instead."

    // Suppress the warning:
    return;
  }

  originalEmitWarning(warning, type, code, ctor);
}

It's a little hacky, but as far as I know NodeJS doesn't provide any other facility for fine-grained filtering of warnings. If you move the DEP0097 constant into a config file, you could make this into a general feature of just-task. As cool as it would be for Rush to provide this out of box, such suppressions should be scoped as narrowly as possible, and individual NodeJS processes are not really Rush's business.

@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants