Skip to content

chore: update imports for node 24 #6513

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

mhahn2003
Copy link
Contributor

@mhahn2003 mhahn2003 commented Jul 22, 2025

This is done for Node.js 24 compatibility

  • Replace import assert = require('assert') with import assert from 'assert' to resolve "TypeScript import equals declaration is not supported in strip-only mode" error

  • Use type-only imports to resolve ESM issues

  • Replace const ... = require('') to import statements

  • Move type classifiers out of constructors

TICKET: CAAS-325

@mhahn2003 mhahn2003 force-pushed the CAAS-325-imports branch 2 times, most recently from 5de4b91 to d67d7a2 Compare July 23, 2025 15:44
@mhahn2003 mhahn2003 marked this pull request as ready for review July 23, 2025 16:47
@mhahn2003 mhahn2003 requested review from a team as code owners July 23, 2025 16:48
ppongbitgo
ppongbitgo previously approved these changes Jul 23, 2025
@andrew-scott-fischer
Copy link
Contributor

andrew-scott-fischer commented Jul 23, 2025

As a separate item to action, we should probably move from assert to assert/strict. see here

@mhahn2003 mhahn2003 force-pushed the CAAS-325-imports branch 2 times, most recently from 5e4c0a7 to 95317fd Compare July 23, 2025 20:50
@mhahn2003 mhahn2003 requested a review from ppongbitgo July 23, 2025 23:04
lcovar
lcovar previously approved these changes Jul 23, 2025
@mhahn2003 mhahn2003 dismissed stale reviews from lcovar and ppongbitgo via 67af8e2 July 24, 2025 17:57
@mhahn2003 mhahn2003 force-pushed the CAAS-325-imports branch 2 times, most recently from 67af8e2 to 36d3798 Compare July 24, 2025 18:30
@andrew-scott-fischer andrew-scott-fischer marked this pull request as draft July 24, 2025 18:39
@mhahn2003 mhahn2003 marked this pull request as ready for review July 24, 2025 21:52
MessageOptions,
MessageStandardType,
} from '@bitgo/sdk-core';
import * as sdkCore from '@bitgo/sdk-core';
Copy link
Contributor

@prabh-codes prabh-codes Jul 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Less effective, often pulls in more code than necessary.

import { BaseMessage, BaseMessageBuilder, BaseMessageBuilderFactory, IMessage, IMessageBuilder, MessageOptions, MessageStandardType, } from '@bitgo/sdk-core'; was good. While you can import type separately for readability but importing by first approach is still considered as good due to:

  1. Optimal Tree Shaking
  2. Clear Intent
  3. Simplicity

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prabh-codes This was done more so to resolve this type of error rather than readability:

@bitgo/abstract-eth: file:///Users/michaelhahn/bitgo/BitGoJS/modules/abstract-eth/test/unit/token.ts:1
@bitgo/abstract-eth: import { TestBitGo, TestBitGoAPI } from '@bitgo/sdk-test';
@bitgo/abstract-eth: ^^^^^^^^^^^^
@bitgo/abstract-eth: SyntaxError: Named export 'TestBitGoAPI' not found. The requested module '@bitgo/sdk-test' is a CommonJS module, which may not support all module.exports as named exports.
@bitgo/abstract-eth: CommonJS modules can always be imported via the default export, for example using:
@bitgo/abstract-eth: import pkg from '@bitgo/sdk-test';
@bitgo/abstract-eth: const { TestBitGo, TestBitGoAPI } = pkg;
@bitgo/abstract-eth: at ModuleJob.#_instantiate (node:internal/modules/esm/module_job:254:21)
@bitgo/abstract-eth: at async ModuleJob.run (node:internal/modules/esm/module_job:362:5)
@bitgo/abstract-eth: at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:665:26)
@bitgo/abstract-eth: at async formattedImport (/Users/michaelhahn/bitgo/BitGoJS/node_modules/mocha/lib/nodejs/esm-utils.js:7:14)
@bitgo/abstract-eth: at async Object.exports.requireOrImport (/Users/michaelhahn/bitgo/BitGoJS/node_modules/mocha/lib/nodejs/esm-utils.js:48:32)
@bitgo/abstract-eth: at async Object.exports.loadFilesAsync (/Users/michaelhahn/bitgo/BitGoJS/node_modules/mocha/lib/nodejs/esm-utils.js:103:20)
@bitgo/abstract-eth: at async singleRun (/Users/michaelhahn/bitgo/BitGoJS/node_modules/mocha/lib/cli/run-helpers.js:125:3)
@bitgo/abstract-eth: at async Object.exports.handler (/Users/michaelhahn/bitgo/BitGoJS/node_modules/mocha/lib/cli/run.js:374:5)
@bitgo/abstract-eth: error Command failed with exit code 1.
@bitgo/abstract-eth: info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Copy link
Contributor

@andrew-scott-fischer andrew-scott-fischer Jul 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error you post is related to @bitgo/sdk-test, not @bitgo/sdk-core. Regardless, @prabh-codes is correct. You should go fix this in the module you are importing from, rather than just importing everything with a * wild card.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants