update all dependencies #2693

ac10n · 2023-10-10T19:26:08Z

closes #2612
closes #2435
closes #2396

Your code builds cleanly without any errors or warnings
You have run the linter against the changes
You have added unit tests (if relevant/appropriate)
You have added integration tests (if relevant/appropriate)
All public methods or types have TypeDoc coverage with a complete description, and ideally an @example
You have added or updated corresponding documentation
If relevant, you have written a first draft summary describing the change for inclusion in Release Notes.

In this PR, please also make sure:

You have linked this PR to the issue by putting closes #TICKETNUMBER in the description box (when applicable)
You have added a concise description on your changes

Release Note Draft Snippet

All dependencies are updated to the latest versions. The only exception is axios, which will later be replace with fetch.
Also, the node version is upgraded from 16 to 18.

netlify · 2023-10-10T19:26:14Z

✅ Deploy Preview for taquito-test-dapp ready!

Name	Link
🔨 Latest commit	`88a8d59`
🔍 Latest deploy log	https://app.netlify.com/sites/taquito-test-dapp/deploys/652e1ab392ae9a00089b1f8a
😎 Deploy Preview	https://deploy-preview-2693--taquito-test-dapp.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

github-actions · 2023-10-10T23:28:57Z

A new deploy preview is available on Netlify at https://321a4da--tezostaquito.netlify.app

github-actions · 2023-10-11T18:26:21Z

New packages have been deployed to the preview repository at https://npm.preview.tezostaquito.io/.

Published packages:

npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/
npm i @taquito/[email protected] --registry https://npm.preview.tezostaquito.io/

.github/workflows/deploy_website.yml

dsawali · 2023-10-12T23:21:47Z

.github/workflows/main.yml

@@ -17,7 +17,6 @@ jobs:
      fail-fast: false
      matrix:
        node:
-          - 'lts/gallium'


i don't think we should remove gallium completely, even if gallium is maintenance LTS, it still is LTS. So we'd want to make sure it still works with gallium

Sure. I'll check to see how much things need to go back to support gallium.

There is a problem here: When we test Taquito on 16, we also build it. But new version of some of our dependencies require node >=18. Examples are rollup and lint-staged.
I think one good way to move faster is:

Move the build environment with the lastest lts (or even latest current)

Keep the engine version in package.json support the oldest supported lts

In CI, build with a recent node version, but test with all supported node versions

This allows us to move faster, but make sure we keep supporting the node versions we still care about.

dsawali · 2023-10-12T23:22:19Z

.github/workflows/main.yml

@@ -77,7 +76,7 @@ jobs:
    - uses: actions/checkout@v3
    - uses: actions/setup-node@v3
      with:
-        node-version: lts/gallium
+        node-version: lts/hydrogen


can we be certain that flextesa builds are good with hydrogen? iirc we used to have an issue with it

We are getting more flextesa errors. Maybe this explains it. What is a possible reason for the relation?

dsawali · 2023-10-12T23:23:45Z

README.md

-| v16 LTS/Gallium  |    ✅      |
-| 17.3.x           |    ✅      |
+| v16.13.1         |    ❌      |
+| v16 LTS/Gallium  |    ❌      |


I think we should still support gallium until Node themselves stops maintenance LTS on gallium

dsawali · 2023-10-12T23:30:26Z

integration-tests/contract-call-method-manual-steps.spec.ts

-        });
-
-        return object;
+    test('Estimates, forges, signs, obtains the operation hash and injects the operation', async () => {


I'm a little bit confused to read what the changes actually are here in this file

I suggest you set the option "hide whitespaces"

dsawali · 2023-10-12T23:37:48Z

integration-tests/package.json

@@ -12,7 +12,7 @@
  },
  "version": "17.3.1",
  "dependencies": {
-    "@ledgerhq/devices": "6.20.0",
+    "@ledgerhq/devices": "8.0.7",


are we able to test these changes?

@michaelkernaghan

@ledgerhq/devices seems not used anywhere 😅

@hui-an-yang Should we remove it then?

i'd love to remove it, might still want to be safe to test it's working as expected

dsawali · 2023-10-12T23:48:00Z

packages/taquito-beacon-wallet/rollup.config.ts

@@ -22,8 +21,5 @@ export default {
    json(),
    // Compile TypeScript files
    typescript({ tsconfig: './tsconfig.prod.json', useTsconfigDeclarationDir: true }),
-
-    // Resolve source maps to the original source
-    sourceMaps(),


do we not need these anymore?

I need to verify this. I'll compare packaged contents before and after this change to make sure nothing is being broken. The reason for this change is that the rollup-plugin-sourcemap is not updated for a long time, and depends on an older version of rollup.

I checked, and seems that the files are the same before and after this change

dsawali · 2023-10-12T23:55:20Z

packages/taquito-remote-signer/src/taquito-remote-signer.ts

@@ -88,7 +88,13 @@ export class RemoteSigner implements Signer {

  private createURL(path: string) {
    // Trim trailing slashes because it is assumed to be included in path
-    return `${this.rootUrl.replace(/\/+$/g, '')}${path}`;
+    // the regex solution is prone to ReDoS. Please see: https://stackoverflow.com/questions/6680825/return-string-without-trailing-slash#comment124306698_6680877


good call on fixing this

dsawali · 2023-10-13T00:03:35Z

packages/taquito/src/constants.ts

+  REVEAL_TZ3: 2000,
+  REVEAL_TZ4: 2000,
+};
+export const DEFAULT_FEE = {


make sure to update all references to the type change from enum to object

Interestingly, there seemed to be no need to change. And frankly, I think object is more relevant here.

apps/taquito-test-dapp/package.json

hui-an-yang · 2023-10-16T18:04:31Z

website/src/theme/CodeBlock/index.js

@@ -84,7 +83,8 @@ export default ({
      const { Parser, packDataBytes } = await import('@taquito/michel-codec');
      const { RpcClient } = await import('@taquito/rpc');
      const { SaplingToolkit, InMemorySpendingKey, InMemoryViewingKey } = await import('@taquito/sapling');
-
+      const TransportWebHID = (await import("@ledgerhq/hw-transport-webhid")).default;


curious the reason behind this change have the package update what it export?

I'm not sure, but as we update packages, some of them fail to be unable to be imported normally in the website. Should have something to do with ESM vs commonJS packaging

If this is the only way it works, I understand that we have to
Not a blocker just trying to understand the root cause, unsure esm/commonjs is relevant in the context since @ledgerhq/hw-transport-webhid didn't upgrade

tsconfig.typedoc.json

packages/taquito/signature.json

packages/taquito-utils/src/taquito-utils.ts

packages/taquito-rpc/src/taquito-rpc.ts

hui-an-yang

Left some comments to address, many thanks for updating all dependencies!
For next time will appreciate that we can separate a pr for style only changes will be easier for teamwork.

ac10n requested review from dsawali and hui-an-yang October 11, 2023 17:52

ac10n force-pushed the 2023-oct-update-all-dependencies branch 3 times, most recently from 599f0d3 to 56c5726 Compare October 12, 2023 20:08

ac10n marked this pull request as ready for review October 12, 2023 20:42

dsawali reviewed Oct 13, 2023

View reviewed changes

ac10n added 16 commits October 16, 2023 09:08

incomplete: update all dependencies

8681afe

fix build commands

c3d308a

chore: build succeeds

2716700

chore: unit tests are working

f512e27

chore: integration tests are working

8c60ac4

chore: downgrade some dependencies in website

9e745d7

chore: test dapp is working

7d65398

chore: update all dependencies again

affe534

chore: format all changed files

a800923

chore: remove unneeded rollup plugin

0085471

chore: update node version in CI

72565ed

chore: fix lint errors

0090114

chore: improvements to website

2b29fcf

chore: remoce polynomial regular expression

bed0a9f

chore: trying to fix website build in ci

38bef45

chore: remove the outdated rollup-plugin-sourcemap

09663a5

ac10n force-pushed the 2023-oct-update-all-dependencies branch from 5145339 to 09663a5 Compare October 16, 2023 16:25

chore: update all dependencies again

45c64ac