Need to patch the standard library

[benjamingr]

Hey,

Thanks for the polyfill!

util.promisify doesn't only add the custom promisify argument - it also patches the standard library methods with a custom promisification property. This is useful for things like fs.exists.

[ljharb]

The PR that added it does do that, yes - i wasn't sure that would make sense for this library to handle.

Since the default behavior of this lib is not to mutate the environment, and to be used as an imported function, I'm not sure what the best way to cover those custom implementations would be.

[RyanZim]

Hi everyone! I'm one of the fs-extra maintainers. We've recently added promise support, and we're thinking of switching to util.promisify and returning objects for the methods that callback with multiple results.

I'm thinking; what if we created a library that returned proxies to the core methods with [util.promisify.custom] added as needed? Just one idea.

[ljharb]

@RyanZim do you mean, such that it can be correctly passed into util.promisify? That seems pretty useful (especially if it allowed you to optionally and explicitly shim the core methods, by installing those custom promisify implementations)

[RyanZim]

do you mean, such that it can be correctly passed into util.promisify?

Yeah.

Not sure how much work this would be; an explicit shim by default might be easier. On that note, since shimming the core methods would simply be adding a symbol property, what side effects could result from globally shimming core? (I'm aware that global shims are generally not the greatest idea due to side effects, but I'm not sure if/what the side affects here would be.) Sorry if this is a noob question, I don't have much experience polyfilling things.

[ljharb]

@RyanZim i think maybe the easiest would be, exporting a function that takes a core method, and returns a promisified version of it - ie, detecting which method it is, and custom-promisifying it with util.promisify, an returning it. Separately, it'd provide an entry point to install the custom symbols on all the core methods.

Globally mutating things is generally not good, but in this case it's making older nodes safely match newer ones, so you're ok :-)

[RyanZim]

@ljharb I've discussed this with @jprichardson and we're leaning in favor of not using util.promsify, but manually shimming promise implementations to return objects for the few fs methods that need help. At least until Node 8+ is more common in the ecosystem. We want to keep our dependency tree light, and adding tons of polyfills doesn't help with that. So chances are, fs-extra won't actually be using this; for awhile anyhow.

[ljharb]

That's certainly your call, but node 8 will be LTS in october, so anything you fail to publish in advance of that will be a missed opportunity.

[jprichardson]

@ljharb will you elaborate on what you mean by "missed opportunity"?

[ljharb]

@jprichardson I mean that "being compatible with both node 4, 6, and 8, in a way that lifts out easily when node 4 and 6 are no longer supported" is going to be super critical once node 8 hits LTS, and a package that makes this easier (like util.promisify, or potentially like fs-extra) is going to get a usage bump as a result. If your package doesn't ship it, another one will, so while it's not necessarily a missed opportunity for the ecosystem (assuming whoever builds it is as responsible a maintainer as you are), it indeed might be a missed opportunity for fs-extra.

[RyanZim]

@ljharb To clarify, we're planning on returning objects like util.promisify, just not actually using util.promisify. When Node 8 hits LTS, we might reconsider; but I'm not a fan of pilling on extra polyfills.

We didn't add promise support until we only supported Node 4+; the ecosystem did produce a few wrapper libraries for this, and I actually helped maintain one of them. Then when fs-extra did add promise support, the wrappers were deprecated in favor of fs-extra.

[ljharb]

Ah, thanks for clarifying. Node had promises since 0.12 so I'm not sure why you'd have needed to wait for v4 tho :-)

[RyanZim]

not sure why you'd have needed to wait for v4 tho :-)

Just a lack of developer time to get it done. 0.10 & 0.12 EOLed in quick succession.