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

ExperimentalWarning: The fs.promises API is experimental #392

Closed
ofrobots opened this issue Jun 25, 2018 · 16 comments
Closed

ExperimentalWarning: The fs.promises API is experimental #392

ofrobots opened this issue Jun 25, 2018 · 16 comments
Assignees
Labels
🚨 This issue needs some love. triage me I really want to be triaged. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@ofrobots
Copy link
Contributor

From @kriskate on June 25, 2018 21:56

Environment details

  • OS: macOS High Sierra
  • Node.js version: 10.1.0
  • npm version: 5.6.0
  • @google-cloud/logging-winston version: 0.9.0

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 ofrobots added needs more info This issue needs more information from the customer to proceed. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Jun 25, 2018
@ofrobots
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@ofrobots ofrobots added priority: p2 Moderately-important priority. Fix may not be included in next release. and removed needs more info This issue needs more information from the customer to proceed. labels Jun 25, 2018
@ofrobots
Copy link
Contributor Author

ofrobots commented Jun 25, 2018

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
Copy link
Contributor Author

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

@JustinBeckwith
Copy link
Contributor

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
Copy link
Contributor Author

The warning is disconcerting for users.

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

@kriskate
Copy link

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

Alternatively, maybe using esmodules for when compiling with tsc might help, according to microsoft/TypeScript#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
Copy link
Contributor Author

@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
Copy link
Contributor Author

The experimentalWarning seems to be fixed with Node 10.2.0+ due to nodejs/node#20403.

@JustinBeckwith
Copy link
Contributor

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

@ofrobots
Copy link
Contributor Author

Actually I am not sure any more given then observation in #392 (comment). If this is no longer a problem with recent Node 10, I am not sure if there is any change necessary.

@kriskate
Copy link

kriskate commented Jun 28, 2018

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
Copy link
Contributor Author

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.

@ofrobots ofrobots removed the priority: p2 Moderately-important priority. Fix may not be included in next release. label Jun 28, 2018
@kriskate
Copy link

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 😃🍻

@Ore4444
Copy link

Ore4444 commented Oct 21, 2018

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

@yoshi-automation yoshi-automation added 🚨 This issue needs some love. triage me I really want to be triaged. labels Apr 6, 2020
@JustinBeckwith JustinBeckwith self-assigned this Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚨 This issue needs some love. triage me I really want to be triaged. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

5 participants