googleapis / google-auth-library-nodejs

🔑 Google Auth Library for Node.js
Apache License 2.0
1.7k stars 374 forks source link

ExperimentalWarning: The fs.promises API is experimental #392

Closed ofrobots closed 6 years ago

ofrobots commented 6 years ago

From @kriskate on June 25, 2018 21:56

Environment details

Steps to reproduce

import { LoggingWinston } from'@google-cloud/logging-winston'

I receive the console error (node:27103) ExperimentalWarning: The fs.promises API is experimental which is unfortunate because I'm redirecting the console to the logger and always get this error in Stackdriver as well

Copied from original issue: googleapis/nodejs-logging-winston#105

ofrobots commented 6 years ago

Hi @kriskate! Thanks for the bug report. Given that you're using ES6 modules can you also provide details on how you are using them? E.g. are you using babel or TypeScript, and if so, what versions.

Secondly can you run with the Node.js --trace-warnings flag so that we can see the stack trace leading to the warning? node --trace-warnings index.js.

ofrobots commented 6 years ago

From @kriskate on June 25, 2018 23:39

Hey, thanks for the prompt response. requiring the modules has the same result let LoggingWinston = require('@google-cloud/logging-winston')

(node:33405) ExperimentalWarning: The fs.promises API is experimental
    at Object.get [as promises] (fs.js:84:15)
    at __importStar (~/node_modules/google-auth-library/build/src/auth/googleauth.js:58:96)
    at Object.<anonymous> (~/node_modules/google-auth-library/build/src/auth/googleauth.js:65:10)
    at Module._compile (internal/modules/cjs/loader.js:678:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:689:10)
    at Module.load (internal/modules/cjs/loader.js:589:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:528:12)
    at Function.Module._load (internal/modules/cjs/loader.js:520:3)
    at Module.require (internal/modules/cjs/loader.js:626:17)
    at require (internal/modules/cjs/helpers.js:20:18)
ofrobots commented 6 years ago

Thanks! I believe this is an issue in the google-auth-library module so I am going to move the issue there.

ofrobots commented 6 years ago

This is due to use of import * from 'fs' here: https://github.com/google/google-auth-library-nodejs/blob/b82d2c401089390813ee9edcda634ec14853e36e/src/auth/googleauth.ts#L20-L26

We should avoid using import * for Node built-in module now that we are using TS synthetic imports.

ofrobots commented 6 years ago

We should also look into a lint rule to prevent this from happening elsewhere.

JustinBeckwith commented 6 years ago

I actually changed this back to a * import in #382 🤣

Synthetic imports on node core libraries only seem to work if downstream TypeScript users also have enabled synthetic imports.

The warning is harmless FWIW.

ofrobots commented 6 years ago

The warning is disconcerting for users.

I see #382. We should drop esModuleInterop from our TS config. WDYT?

kriskate commented 6 years ago

Wouldn't importing from fs/promises solve the issue?

Alternatively, maybe using esmodules for when compiling with tsc might help, according to https://github.com/Microsoft/TypeScript/issues/22851. The thread also says something about __importStar, which is used to load the credentials I'm passing through keyFilename. Please have a look.

ofrobots commented 6 years ago

@kriskate we actually do not use fs.promises at all. The warning is being generated due to the implementations details of how TypeScript generates code for import *. It iterates all properties of the fs module object and in doing so it accesses fs.promises even though we don't need to use it.

Similarly, we cannot use esmodules in the target setting because we support users running with Node 6.

ofrobots commented 6 years ago

The experimentalWarning seems to be fixed with Node 10.2.0+ due to https://github.com/nodejs/node/pull/20403.

JustinBeckwith commented 6 years ago

Just so I'm clear - @ofrobots is the desired fix here then named imports from fs?

ofrobots commented 6 years ago

Actually I am not sure any more given then observation in https://github.com/google/google-auth-library-nodejs/issues/392#issuecomment-400468336. If this is no longer a problem with recent Node 10, I am not sure if there is any change necessary.

kriskate commented 6 years ago

As a solution to my problem, which was that Winston was logging this error to Stackdriver through the plug-in, I was using previous version of node (pre-promises) for my builds; I've now switched to node version 10.5.0, and as stated by @ofrobots the experimentalWarning is not shown anymore.

For me, the issue is not there anymore, although there might still be CIs out there that use a node version which outputs the error. However, the solution is simple: just upgrade node to a post-experimental promise version (^v10.2.0). Can be closed if you ask me.

PS: This doesn't change the fact that building TS projects with the same configuration and style of the projects this issue has been moved through might add unnecessary methods/ blocks of code to the project.

ofrobots commented 6 years ago

might add unnecessary methods/ blocks of code to the project.

@kriskate can you elaborate on what you mean here? While I do think it is slightly better to use named imports rather than doing import * from 'fs', but doing so doesn't really add any unnecessary methods/code to a project.

The spurious experimental warning was an issue in Node that has since been fixed, so I do not think there is anything more actionable here.

kriskate commented 6 years ago

Well, I haven't dwelled into all the TS hype, but from what I can tell, somehow the combination of config flags and import style used for compiling google-auth-library-nodejs accessed fs/promises (even if not used), otherwise I can't tell why the error appeared.

Anyway, thank you very much for helping me fix this, not to mention for developing these utils :smiley::beers:

Ore4444 commented 5 years ago

For anyone this might be useful for: https://stackoverflow.com/questions/50441890/experimentalwarning-the-fs-promises-api-is-experimental/52915615#52915615