nodejs-storage icon indicating copy to clipboard operation
nodejs-storage copied to clipboard

refactor(deps)!: Remove `html-entities` dependency

Open danielbankhead opened this issue 3 years ago • 14 comments

Remove ent & @types/ent dependencies - they do not seem necessary given the context of its usage.

Perhaps decodeURI would work as an alternative.

danielbankhead avatar May 05 '22 17:05 danielbankhead

I do not believe this to be easy to remove. The reason it is imported is that under certain situations (rate limiting if I remember correctly) the GCS API will return a response in HTML. Currently Ent is used to decode the returned HTML. I spoke with other client library owners and while they do not parse the HTML like Node does, if we were to remove this it would be a breaking change. We have two choices, close this issue as will not fix or move it until our next major release which is tentatively scheduled for Spring 2023.

ddelgrosso1 avatar Aug 02 '22 15:08 ddelgrosso1

Hi, I think this needs revisting. ent relies on the deprecated punycode module. This is now causing issues in Node v21.x.

luc122c avatar Nov 18 '23 20:11 luc122c

Hi @luc122c thanks for the heads up. I will take another look and see if there is a way we can remove ent without causing a breaking change.

ddelgrosso1 avatar Nov 19 '23 14:11 ddelgrosso1

Not sure if it's a drop in replacement, but perhaps https://www.npmjs.com/package/entities could be used to replace ent (which hasn't been updated in 9 years).

drichardson avatar Jan 11 '24 01:01 drichardson

@drichardson thanks for the heads up. I will poke around in entities to see if it meets our needs.

ddelgrosso1 avatar Jan 11 '24 14:01 ddelgrosso1

@ddelgrosso1 Whoops I just saw your note after making a branch using a different library called he which is pretty old and boring which seemed good for this 😄.

drichardson avatar Jan 11 '24 18:01 drichardson

@luc122c @ddelgrosso1 I'm having trouble with this in today's latest version.

FROM node:latest

cfdavidpetter avatar Apr 18 '24 14:04 cfdavidpetter

@cfdavidpetter this issue for removing ent which has not been done. If you are having another issue please feel free to open a new one.

ddelgrosso1 avatar Apr 18 '24 14:04 ddelgrosso1

@ddelgrosso1 my problem is this:

(node:8) [DEP0040] DeprecationWarning: The punycode module is deprecated. Please use a userland alternative instead. at node:punycode:3:9 at BuiltinModule.compileForInternalLoader (node:internal/bootstrap/realm:398:7) at BuiltinModule.compileForPublicLoader (node:internal/bootstrap/realm:337:10) at loadBuiltinModule (node:internal/modules/helpers:104:7) at Module._load (node:internal/modules/cjs/loader:999:17) at Module.require (node:internal/modules/cjs/loader:1230:19) at require (node:internal/modules/helpers:179:18) at Object.<anonymous> (/app/node_modules/whatwg-url/lib/url-state-machine.js:2:18) at Module._compile (node:internal/modules/cjs/loader:1368:14) at Module._extensions..js (node:internal/modules/cjs/loader:1426:10)

dependencies:
"@google-cloud/storage": "^7.10.0",

cfdavidpetter avatar Apr 18 '24 14:04 cfdavidpetter

Thanks @cfdavidpetter we are aware of the deprecation of punycode and plan to remove ent in the next major version of the storage library.

ddelgrosso1 avatar Apr 18 '24 15:04 ddelgrosso1

Related:

  • https://github.com/googleapis/nodejs-common/pull/813

danielbankhead avatar Apr 27 '24 00:04 danielbankhead

Thanks @danielbankhead this is probably a good stop gap before next major version where I hope to remove the dependency altogether.

ddelgrosso1 avatar Apr 29 '24 13:04 ddelgrosso1

https://github.com/googleapis/nodejs-storage/pull/2451

ddelgrosso1 avatar Apr 29 '24 13:04 ddelgrosso1

Ent has been replaced with html-entities for the time being. I still intend to remove this functionality in the next major version. Going to update the title of this issue accordingly.

ddelgrosso1 avatar Apr 29 '24 13:04 ddelgrosso1