Skip to content

Fix default exports #3248

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

Conversation

GalMunGral
Copy link

Context

Trying to port the examples to TypeScript produces the following errors:
Screenshot 2025-04-29 at 2 50 15 PM

It turns out that default exports are misused in many places in the .d.ts files:

  1. Since the original .js files don’t have types, default exports should always be values. But in some cases, an interface declaration is incorrectly assigned as the default export, rather than a value that conforms to that interface. This leads to the error shown in the screenshot.
  2. Most files correctly default-export a value and separately export the interface type as a named export. However, other .d.ts files mistakenly use the default import syntax to import interfaces — which resolves to the exported const values instead of the type definitions.

Results

Screenshot 2025-04-29 at 3 20 38 PM The type errors related to default exports are resolved.

Changes

  • Changed import/export statements in in multiple .d.ts files.
  • Added a typecheck npm script and a tsconfig.json to ensure the .d.ts files are consistent with each other.
  • Documentation and TypeScript definitions were updated to match those changes (obviously)

PR and Code Checklist

  • semantic-release commit messages
  • Run npm run reformat to have correctly formatted code

Testing

  • This change adds or fixes unit tests
  • Tested environment:
    • vtk.js:
    • OS:
    • Browser:

No implementation has changed; this should only affect TypeScript-aware IDEs and build tools.

@finetjul finetjul requested review from daker and floryst April 30, 2025 16:11
@finetjul finetjul added the module: typescript Issues related to typescript definitions label Apr 30, 2025
@daker
Copy link
Collaborator

daker commented May 1, 2025

i don't know what's your use case here but they are imported like this:

import DataAccessHelper from '@kitware/vtk.js/IO/Core/DataAccessHelper';
// import '@kitware/vtk.js/IO/Core/DataAccessHelper/LiteHttpDataAccessHelper'; // Just need HTTP
import '@kitware/vtk.js/IO/Core/DataAccessHelper/HttpDataAccessHelper'; // HTTP + zip
// import '@kitware/vtk.js/IO/Core/DataAccessHelper/HtmlDataAccessHelper'; // html + base64 + zip
// import '@kitware/vtk.js/IO/Core/DataAccessHelper/JSZipDataAccessHelper'; // zip

@GalMunGral
Copy link
Author

i don't know what's your use case here but they are imported like this:

import DataAccessHelper from '@kitware/vtk.js/IO/Core/DataAccessHelper';
// import '@kitware/vtk.js/IO/Core/DataAccessHelper/LiteHttpDataAccessHelper'; // Just need HTTP
import '@kitware/vtk.js/IO/Core/DataAccessHelper/HttpDataAccessHelper'; // HTTP + zip
// import '@kitware/vtk.js/IO/Core/DataAccessHelper/HtmlDataAccessHelper'; // html + base64 + zip
// import '@kitware/vtk.js/IO/Core/DataAccessHelper/JSZipDataAccessHelper'; // zip

I saw that HttpDataAccessHelper is imported and used here, for example:

import HttpDataAccessHelper from '@kitware/vtk.js/IO/Core/DataAccessHelper/HttpDataAccessHelper';
HttpDataAccessHelper.fetchBinary(...)

So should this be

import DataAccessHelper from '@kitware/vtk.js/IO/Core/DataAccessHelper';
import '@kitware/vtk.js/IO/Core/DataAccessHelper/HttpDataAccessHelper';
DataAccessHelper.get('http').fetchBinary(...)

instead? Or are both usages valid?

Copy link
Collaborator

@floryst floryst left a comment

Choose a reason for hiding this comment

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

Thanks for adding a typecheck command!

Instead of exporting a declared const _default and changing imports to use curly braces, you can declare a value with the same name as the interface (see the following example with HtmlDataAccessHelper), and typescript will infer the type or value:

// HtmlDataAccessHelper/index.d.ts
declare const HtmlDataAccessHelper: HtmlDataAccessHelper
export default HtmlDataAccessHelper

// other.d.ts
import HtmlDataAccessHelper from '...'
// HtmlDataAccessHelper can be used as a type or a value

Comment on lines 18 to 19
toNativeType(str: string): any;
extractURLParameters(
Copy link
Collaborator

Choose a reason for hiding this comment

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

To simplify the typings, you can just do the following:

toNativeType: typeof toNativeType,
extractURLParameters: typeof extractURLParameters,

Copy link
Author

Choose a reason for hiding this comment

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

Thanks--this is neat!

@GalMunGral
Copy link
Author

@floryst Thanks for the suggestions! I've updated my changes.

However, removing the curly braces didn't work for me—I got errors like this from tsc:

error TS2749: 'JSZipDataAccessHelper' refers to a value, but is being used as a type here. Did you mean 'typeof JSZipDataAccessHelper'?

@GalMunGral GalMunGral requested a review from floryst May 1, 2025 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: typescript Issues related to typescript definitions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants