fix: replace isBrowser() with home made feature detection #1054
Conversation
google-cla
bot
added
the
cla: yes
| } from './featureDetection'; | ||
|
|
||
| if (!hasTextEncoder() || !hasTextDecoder()) { | ||
| if (isNodeJS()) { |
There was a problem hiding this comment.
I generally don't like the idea of trying to figure out if something is Node.js or not. The philosophy behind feature detection is to say which browser/env it is doesn't matter, and instead focus on the availability of the API itself. Things like electron and webpack make figuring out "is this node?" a fools errand. All that having been said - I don't have a ready made suggestion on how to do this better :)
There was a problem hiding this comment.
I thought about this. There are several use cases of isNodeJS(), what I actually need is hasUtil() (can require("util"), hasEmitWarning(), and something for "can authenticate GoogleAuth instance by accessing filesystem". Checking for process.emitWarning is straightforward (and it might make sense to have a separate "feature" for it), but I don't know a good way to check if require('util').TextEncoder and require('util').TextDecoder are available - so I'm just assuming here that if it's Node.js, it must be able to do require("util"). Do you think there might be a better way?
(and yes, we'll drop Node v10 and this particular if on line 29 will disappear)
🤖 I have created a release \*beep\* \*boop\* --- ## [2.18.0](https://www.github.com/googleapis/gax-nodejs/compare/v2.17.1...v2.18.0) (2021-07-13) ### Features * make OperationsClient closeable ([#1047](https://www.github.com/googleapis/gax-nodejs/issues/1047)) ([2dbba29](https://www.github.com/googleapis/gax-nodejs/commit/2dbba29dde552fb35c275a4a44b06fb4698eb5cf)) * support map handle for DIREGAPIC Pagination ([#1052](https://www.github.com/googleapis/gax-nodejs/issues/1052)) ([faab4c6](https://www.github.com/googleapis/gax-nodejs/commit/faab4c652c4943fc18c792995180bf59dbd5c7bc)) ### Bug Fixes * make pagination work for empty responses ([#1043](https://www.github.com/googleapis/gax-nodejs/issues/1043)) ([cbe2d3f](https://www.github.com/googleapis/gax-nodejs/commit/cbe2d3f9de4ec01e8e61699b5fa6bf7b34b870a5)) * replace isBrowser() with home made feature detection ([#1054](https://www.github.com/googleapis/gax-nodejs/issues/1054)) ([2c8e56d](https://www.github.com/googleapis/gax-nodejs/commit/2c8e56d5812af7b08ff6d68169d1d8ea325e03c2)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Fixes #1046 by replacing general
isBrowser()check with targeted feature detection.Cc: @schmidt-sebastian