Fix: Update nns-js candid types

[lmuntaner]

Motivation

The field is_genesis has been reverted. Therefore, I updated the candid files to the latest commit for nns-js.

Changes

Automatic changes

• .did files with ./scripts/import-candid.
• Candid related js and type files with ./scripts/compile-idl-js.

Manual changes

• Fix converters.

Tests

• Remove field from neuron mock

Todos

• Add entry to changelog (if necessary).
  I removed a previous entry.

[lmuntaner]

@dskloetd please review

It seems that I had bad luck with the commit I chose in the previous update. The new field was reverted shortly afterwards.

[github-actions]

size-limit report 📦

Path	Size
@dfinity/ckbtc	6.79 KB (0%)
@dfinity/cmc	1.01 KB (0%)
@dfinity/ledger-icrc	2.92 KB (0%)
@dfinity/ledger-icp	14.17 KB (0%)
@dfinity/nns	33.74 KB (+0.6% 🔺)
@dfinity/nns-proto	76.3 KB (0%)
@dfinity/sns	14.89 KB (0%)
@dfinity/utils	3.81 KB (0%)
@dfinity/ic-management	1.94 KB (0%)

[dskloetd]

I see a lot of changes. Way more than just reverting is_genesis.
I think it would be worth installing in nns-dapp locally from path to check if it works (npm run build, npx tsc --noEmit, npm run check).
Then if additional changes are necessary in nns-dapp you can prepare them ready to go.
What do you think?

FYI, I use this script to install ic-js locally. Maybe you can partially reuse it.

set -xe

if [ -z "$1" ]; then
  echo "Specify the path to the ic-js repo." >&2
  exit 1
fi

IC_JS_PATH="$(realpath $1)"

cd frontend
rm -rf node_modules
npm ci

for x in utils ledger-icrc ledger-icp nns-proto nns; do
  npm rm @dfinity/$x && npm i $IC_JS_PATH/packages/$x
done

npm ci

[lmuntaner]

I see a lot of changes. Way more than just reverting is_genesis. I think it would be worth installing in nns-dapp locally from path to check if it works (npm run build, npx tsc --noEmit, npm run check). Then if additional changes are necessary in nns-dapp you can prepare them ready to go. What do you think?

Good point, I tested it and all looks good.

Why would I need to test that I need some extra changes in the tests in advance? What's the benefit? As long as it works to fetch and interact with the canister, the rest can easily be done afterwards.

[dskloetd]

I'm not sure I understand the question. But what I was trying to say is that if it turns out to be complicated to make the changes work in nns-dapp, we want to figure those things out before merging in ic-js so that we don't block other changes or cause breakages for other people trying to upgrade next, while we are still trying to figure out what's wrong.

[lmuntaner]

I'm not sure I understand the question. But what I was trying to say is that if it turns out to be complicated to make the changes work in nns-dapp, we want to figure those things out before merging in ic-js so that we don't block other changes or cause breakages for other people trying to upgrade next, while we are still trying to figure out what's wrong.

I'll make those changes right away. I think that should be the default as you mentioned. I already know that there will be changes needed.

Only if we know there won't be changes, then we can leave it withuot upgrading.

[dskloetd]

Wait, how can you say it all looks good if there are changes required that you haven't made yet?

[lmuntaner]

Because the changes are in the tests. I tested the development server installing nns-js from this branch.

[dskloetd]

OK, but unless you've already made the changes it's always possible that they turn out to be complicated.
And then someone else tries to upgrade next and runs into these problems and gets stuck.
So I think it's worthwhile to try to have the changes ready before merging in ic-js.

[lmuntaner]

OK, but unless you've already made the changes it's always possible that they turn out to be complicated. And then someone else tries to upgrade next and runs into these problems and gets stuck. So I think it's worthwhile to try to have the changes ready before merging in ic-js.

I agree that if it requires changes, they should be done immediately. But just testing them is also not enough. We want a PR to be merged so that another person doesn't get stuck.