Skip to content

NODEJS-680: Update and Automate TypeScript Support #439

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 81 commits into
base: master
Choose a base branch
from

Conversation

SiyaoIsHiding
Copy link
Collaborator

@SiyaoIsHiding SiyaoIsHiding commented Apr 29, 2025

Massive refactoring. Changes can be broken down into the following parts:

  1. Rename files from .js to .ts.
  2. Update import/export syntax to ESM syntax - some index files have to export both named and default to be compatible with the previous require behavior.
  3. Refactor ES5 style classes to ES6 style classes
  4. Cleaning TypeScript errors and eslint errors in core
  5. Fix API exposure - My rationale is: if an API was exposed in previous .d.ts, it's supposed to be exposed. If it has documentation on how a user should use it, it's supposed to be exposed. There are some inconsistencies. Please search //TODO to find them.
  6. Make sure tests pass and set up CI: Replace the use of proxyquire to sinon. Add rollup for bundle, api-extractor to encapusulate, and eslint for linting.

We need to discuss:

  1. Please search //TODO to find the places where I am not sure what to do. Some are breaking changes.
  2. How can we maintain backward compatibility of the dotted type names, like cassandra.errors.DriverError?
    In the current master branch, this is the only way to use types:
import cassandra from 'cassandra-driver';
// ...
client.execute(query, (err: cassandra.errors.DriverError, result: cassandra.types.ResultSet) => {});

Or, with one level down imports

import {errors, types } from 'cassandra-driver';
 client.execute(query, (err: errors.DriverError, result: types.ResultSet) => {});

However, the TypeScript default way, also the way how other libraries expose the types, is to flatten all exported classes.

import {DriverError, ResultSet } from 'cassandra-driver';
 client.execute(query, (err: DriverError, result: ResultSet) => {});

Now how can we maintain backward compatibility to the previous dotted ones? The only workaround I can find is to append such things at the end of our dist/cassandra-driver-public.d.ts

export namespace errors{
    export type DriverError = InstanceType<typeof DriverError>;
}

This is the last step of the bundle script in this PR.
3. How do we maintain backward compatibility of import x = y.z syntax?

import { mapping } from "../../../index";
import Mapper = mapping.Mapper;
const mapper: Mapper = new Mapper();

This currently will raise an error Mapper is a type, but used as a value. I cannot see a solution.

  1. What do we want to do with our documentation? Should we update all code snippets into TypeScript?

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the codebase to support TypeScript and the ESM module system while cleaning up TypeScript and ESLint errors. Key changes include conversion of source files from JavaScript to TypeScript, updates to the import/export syntax and class definitions, and modifications to the ESLint configuration to support TypeScript.

Reviewed Changes

Copilot reviewed 291 out of 294 changed files in this pull request and generated no comments.

Show a summary per file
File Description
lib/client-options.ts Converted legacy require calls to ES module imports and added TypeScript typings to functions.
lib/auth/provider.ts Migrated the authentication provider file from CommonJS to TypeScript with updated syntax.
lib/auth/plain-text-auth-provider.ts Rewritten in TypeScript and refactored class definitions with updated export statements.
lib/auth/no-auth-provider.ts Switched from require to imports while preserving legacy behavior.
lib/auth/index.ts Consolidated exports using both default and named export syntax to support the new module style.
lib/auth/gssapi-client.ts Converted to TypeScript with updated function signatures and unused variable suppression.
lib/auth/dse-plain-text-auth-provider.ts Transitioned from JavaScript to TypeScript following updated coding guidelines.
lib/auth/dse-gssapi-auth-provider.ts Rewritten as TypeScript with proper typings and async patterns for enhanced authentication handling.
lib/auth/base-dse-authenticator.ts Reimplemented in TypeScript to support inheritance and types for DSE-specific authentication logic.
index.ts Updated main entry point to use TypeScript imports/exports and integrate new module components.
.eslintrc.js Updated ESLint configuration to include TypeScript plugins and parser settings.
Files not reviewed (3)
  • .eslintignore: Language not supported
  • .madgerc: Language not supported
  • Jenkinsfile: Language not supported
Comments suppressed due to low confidence (1)

lib/auth/plain-text-auth-provider.ts:67

  • Consider replacing the 'any' type for 'username' with 'string' (and similarly for 'password') to enforce more precise type checking.
username: any;

@SiyaoIsHiding
Copy link
Collaborator Author

I forgot Travis CI. Will fix.

@SiyaoIsHiding
Copy link
Collaborator Author

Contact doc team and see what we should do with JSdoc type definitions v.s. TypeScript type definitions.

/* Excluded from this release type: getCustomTypeSerializers */
/* Excluded from this release type: GraphTypeWrapper */
/* Excluded from this release type: UdtGraphWrapper */
Edge: typeof Edge;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Edge not exported. To fix

@SiyaoIsHiding
Copy link
Collaborator Author

Minimal reproducible case for import x = y.z error

@SiyaoIsHiding
Copy link
Collaborator Author

Optimize the pipeline to only distribute necessary files.

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.

1 participant