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

refactor(deps)!: Remove html-entities dependency #1907

Open
danielbankhead opened this issue May 5, 2022 · 14 comments
Open

refactor(deps)!: Remove html-entities dependency #1907

danielbankhead opened this issue May 5, 2022 · 14 comments
Labels
api: storage Issues related to the googleapis/nodejs-storage API. next major: breaking change this is a change that we should wait to bundle into the next major version priority: p3 Desirable enhancement or fix. May not be included in next release. type: cleanup An internal cleanup or hygiene concern.

Comments

@danielbankhead
Copy link
Contributor

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

Perhaps decodeURI would work as an alternative.

@danielbankhead danielbankhead added type: cleanup An internal cleanup or hygiene concern. priority: p3 Desirable enhancement or fix. May not be included in next release. labels May 5, 2022
@product-auto-label product-auto-label bot added the api: storage Issues related to the googleapis/nodejs-storage API. label May 5, 2022
@danielbankhead danielbankhead changed the title refactor(deps): ent dependency refactor(deps): Remove ent dependency May 5, 2022
@ddelgrosso1
Copy link
Contributor

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.

@shaffeeullah shaffeeullah added the next major: breaking change this is a change that we should wait to bundle into the next major version label Aug 10, 2022
@danielbankhead danielbankhead changed the title refactor(deps): Remove ent dependency refactor(deps)!: Remove ent dependency Aug 12, 2022
@luc122c
Copy link

luc122c commented Nov 18, 2023

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

@ddelgrosso1
Copy link
Contributor

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.

@drichardson
Copy link

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).

@ddelgrosso1
Copy link
Contributor

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

@drichardson
Copy link

@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 😄.

@cfdavidpetter
Copy link

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

FROM node:latest

@ddelgrosso1
Copy link
Contributor

@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.

@cfdavidpetter
Copy link

cfdavidpetter commented Apr 18, 2024

@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",

@ddelgrosso1
Copy link
Contributor

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

@danielbankhead
Copy link
Contributor Author

Related:

@ddelgrosso1
Copy link
Contributor

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

@ddelgrosso1
Copy link
Contributor

#2451

@ddelgrosso1
Copy link
Contributor

ddelgrosso1 commented Apr 29, 2024

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 ddelgrosso1 changed the title refactor(deps)!: Remove ent dependency refactor(deps)!: Remove html-entities dependency Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the googleapis/nodejs-storage API. next major: breaking change this is a change that we should wait to bundle into the next major version priority: p3 Desirable enhancement or fix. May not be included in next release. type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants