Fluentui: Fix deprecation warning

[jdhuntington]

(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]

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]

Does anyone have some simple steps for reproing this warning?

[KevinTCoughlin]

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

Reading https://github.com/nodejs/node/issues/9516#issuecomment-259245301, 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]

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]

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.