Skip to content
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

Published types in @deephaven npm packages are not NodeNext compatible #2273

Open
bmingles opened this issue Oct 29, 2024 · 1 comment
Open
Labels
bug Something isn't working
Milestone

Comments

@bmingles
Copy link
Contributor

bmingles commented Oct 29, 2024

When deploying @deephaven npm packages, we build the packages by running npm run build. This:

  1. Builds the types via the tsc compiler (via the "types" script)
  2. Builds the source code via Babel (via the "build:packages" script)

ESM modules require imports to include the file extension. The Babel compilation seems to produce proper ESM compatible imports by adding the extensions to imports, while the types build does not. This is because TypeScript will not automatically add file extensions, it leaves it up to the author to add the extensions. This causes trouble if a consumer configured for NodeNext attempts to import types.

As an example, when configuring a project as "module": "NodeNext" this fails to find the type:

// It seems the compiler will not find the type that exists deeper than the `index.d.ts` (presumably) due to missing
// file extensions on the imports
import type { Brand } from '@deephaven/utils';

This works:

import type { Brand } from '@deephaven/utils/dist/TypeUtils.d.ts';

Options I'm aware of that would fix this are:

  1. Re-export explicitly named types instead of export type * (not sure why this actually works and doesn't really seem a proper fix)
  2. Convert imports in the source code to include the .js extension. This would be a large refactor to fix the entire codebase. Could be worth targetting specific modules. Also could be worth automating in 1 pass at some point as this seems to be the "proper" format to use. Update imports to use explicit file extensions #223
  3. Figure out if there are any transpiler / compiler options that would add the extensions as part of the build

Lastly, it seems like some scenarios work as-is, so "not ESM compatible" may be a bit too strong of a statement, but it seems we should investigate what the "proper" format should be here for broader compatibility.

@bmingles bmingles added bug Something isn't working triage Issue requires triage labels Oct 29, 2024
@bmingles bmingles changed the title Published types in @deephaven npm packages are not ESM compatible Published types in @deephaven npm packages are not NodeNext compatible Oct 29, 2024
@bmingles
Copy link
Contributor Author

@mofojed mofojed assigned wusteven815 and unassigned wusteven815 Nov 4, 2024
@vbabich vbabich added this to the December 2024 milestone Nov 5, 2024
@vbabich vbabich removed the triage Issue requires triage label Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants