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

remove dep on fast-text-encoder #1546

Closed
jimmywarting opened this issue May 14, 2023 · 4 comments
Closed

remove dep on fast-text-encoder #1546

jimmywarting opened this issue May 14, 2023 · 4 comments
Assignees
Labels
next major: breaking change this is a change that we should wait to bundle into the next major version type: cleanup An internal cleanup or hygiene concern.

Comments

@jimmywarting
Copy link

You only load this when needed.

"fast-text-encoding": "^1.0.0",

But i also see that you have a requirement on NodeJS v12+

"engines": {
"node": ">=12"
},

therefore this npm package could be removed, cuz text encoder/decoder is available globally since NodeJS v11

Also supporting NodeJS v12 is a bit outdated, think you should target LTS and update to something newer such as v16 that is the current LTS right now

So also think you should do:

 "engines": { 
   "node": ">=16" 
 }, 
@ddelgrosso1 ddelgrosso1 added the type: cleanup An internal cleanup or hygiene concern. label May 16, 2023
ddelgrosso1 pushed a commit to ddelgrosso1/google-auth-library-nodejs that referenced this issue May 16, 2023
* fix: add hashes to requirements.txt

and update Docker images so they require hashes.

* fix: add hashes to docker/owlbot/java/src

* Squashed commit of the following:

commit ab7384ea1c30df8ec2e175566ef2508e6c3a2acb
Author: Jeffrey Rennie <rennie@google.com>
Date:   Tue Aug 23 11:38:48 2022 -0700

    fix: remove pip install statements (googleapis#1546)

    because the tools are already installed in the docker image as of googleapis/testing-infra-docker#227

commit 302667c9ab7210da42cc337e8f39fe1ea99049ef
Author: WhiteSource Renovate <bot@renovateapp.com>
Date:   Tue Aug 23 19:50:28 2022 +0200

    chore(deps): update dependency setuptools to v65.2.0 (googleapis#1541)

    Co-authored-by: Anthonios Partheniou <partheniou@google.com>

commit 6e9054fd91d1b500cae58ff72ee9aeb626077756
Author: WhiteSource Renovate <bot@renovateapp.com>
Date:   Tue Aug 23 19:42:51 2022 +0200

    chore(deps): update dependency nbconvert to v7 (googleapis#1543)

    Co-authored-by: Anthonios Partheniou <partheniou@google.com>

commit d229a1258999f599a90a9b674a1c5541e00db588
Author: Alexander Fenster <fenster@google.com>
Date:   Mon Aug 22 15:04:53 2022 -0700

    fix: update google-gax and remove obsolete deps (googleapis#1545)

commit 13ce62621e70059b2f5e3a7bade735f91c53339c
Author: Jeffrey Rennie <rennie@google.com>
Date:   Mon Aug 22 11:08:21 2022 -0700

    chore: remove release config and script (googleapis#1540)

    We don't release to pypi anymore.

* chore: rollback java changes

to move forward with other languages until Java's docker image is fixed
Source-Link: googleapis/synthtool@4826337
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:7fefeb9e517db2dd8c8202d9239ff6788d6852bc92dd3aac57a46059679ac9de

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
@ddelgrosso1 ddelgrosso1 self-assigned this May 16, 2023
@ddelgrosso1 ddelgrosso1 added the next major: breaking change this is a change that we should wait to bundle into the next major version label May 16, 2023
@ddelgrosso1
Copy link

Thank you for opening an issue @jimmywarting. You are correct both node and most modern browsers now support this and there is likely no reason for us to continue to utilize this as a dependency. However, this would be considered a breaking change so it will have to wait until the next major revision of the library. We are planning to drop Node 12 in the near future and move to Node 14 as the minimum version. The change to remove fast-text-encoding will likely go out at the same time.

@jimmywarting
Copy link
Author

any reason for not going with v16 as LTS?
v14 is no longer supported by nodejs

@ddelgrosso1
Copy link

There are a variety of issues with jumping from 12->16. We try to look at metrics on what version customers are currently using and a large percentage are still on 14. While we create a new major version when we bump the minimum version, jumping 12->16 is likely to leave a large portion of the user base unable or unwilling to upgrade. Additionally, we try and make sure we are supporting similar versions with other Cloud products, i.e. Cloud Functions.

@sofisl
Copy link
Contributor

sofisl commented Jul 10, 2023

Removed in d736da3

@sofisl sofisl closed this as completed Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next major: breaking change this is a change that we should wait to bundle into the next major version type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants