-
Notifications
You must be signed in to change notification settings - Fork 3
fix: use explicit .js extensions for ESM-compliant imports #58
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
Conversation
This change fixes import resolution errors that occur when consuming loro-mirror in strict ESM environments. ## Problem The package used directory imports like `from "./schema"` which TypeScript compiles as-is. This causes two distinct failures: 1. **Runtime** (Node.js ESM / Vitest): "Directory import is not supported resolving ES modules" - Node.js ESM requires explicit file extensions. 2. **Compile time** (TypeScript with `moduleResolution: "NodeNext"`): "Module 'loro-mirror' has no exported member 'RootSchemaType'" - TypeScript can't resolve `./schema` to `./schema/index.d.ts` in strict mode. Consumers using lenient resolution modes (`bundler`, legacy `node`) were unaffected, which is why this wasn't caught earlier. ## Solution 1. Changed tsconfig.json to use `module: "NodeNext"` and `moduleResolution: "NodeNext"` 2. Added explicit `.js` extensions to all relative imports ## Sources - "How to Create an NPM Package" by Matt Pocock (Total TypeScript) https://www.totaltypescript.com/how-to-create-an-npm-package - "Is NodeNext right for libraries?" by Andrew Branch (TypeScript team) https://blog.andrewbran.ch/is-nodenext-right-for-libraries-that-dont-target-node-js/ ## Consequences - All relative imports must now include explicit `.js` extensions - TypeScript will error at compile time if extensions are missing - Consumers no longer need workarounds like Vitest's `server.deps.inline` - Works with all TypeScript moduleResolution modes (bundler, node, NodeNext) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this 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 fixes ESM import resolution issues by adding explicit .js extensions to all relative imports and updating TypeScript configuration to use NodeNext module resolution. The changes ensure compatibility with strict ESM environments (Node.js ESM, Vitest) and TypeScript's modern module resolution modes, addressing issue #53 where consumers experienced runtime and compile-time import resolution failures.
Key Changes
- Updated tsconfig.json to use
"module": "NodeNext"and"moduleResolution": "NodeNext"for strict ESM compliance - Added explicit
.jsextensions to all relative imports across source and test files (60+ import statements) - Maintained consistent import patterns across the monorepo (core, react, and jotai packages)
Reviewed changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Updated TypeScript configuration to use NodeNext module resolution for ESM compliance |
| packages/core/src/index.ts | Added .js extensions to schema and core module imports |
| packages/core/src/core/index.ts | Added .js extension to mirror module import |
| packages/core/src/core/mirror.ts | Added .js extensions to loroEventApply, schema, utils, diff, and constants imports |
| packages/core/src/core/utils.ts | Added .js extensions to schema, mirror, and constants imports |
| packages/core/src/core/diff.ts | Added .js extensions to schema, mirror, utils, and constants imports |
| packages/core/src/core/loroEventApply.ts | Added .js extension to utils import |
| packages/core/src/core/loroEventApply.test.ts | Added .js extension to loroEventApply import |
| packages/core/src/schema/index.ts | Added .js extensions to types and validators imports |
| packages/core/src/schema/validators.ts | Added .js extensions to types and utils imports |
| packages/react/src/index.ts | Added .js extension to hooks import |
| packages/react/tests/readme.react.context.test.ts | Added .js extension to index import |
| packages/jotai/tests/readme.jotai.test.tsx | Added .js extension to index import |
| packages/jotai/tests/index.test.tsx | Added .js extension to index import |
| packages/core/tests/*.test.ts | Added .js extensions to all relative imports across 15 test files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
(Copilot PR review didn't impress compared to Codex 🫣) Should have included this to begin with, didn't find a smooth way to get it into a test in the repo without adding more test setup. Before:After:Succeeds. |
|
Thanks! |
Fixes #53
This change fixes import resolution errors that occur when consuming loro-mirror in strict ESM environments.
Problem
The package used directory imports like
from "./schema"which TypeScript compiles as-is. This causes two distinct failures:Runtime (Node.js ESM / Vitest): "Directory import is not supported resolving ES modules" - Node.js ESM requires explicit file extensions.
Compile time (TypeScript with
moduleResolution: "NodeNext"): "Module 'loro-mirror' has no exported member 'RootSchemaType'" - TypeScript can't resolve./schemato./schema/index.d.tsin strict mode.Consumers using lenient resolution modes (
bundler, legacynode) are unaffected.Solution
module: "NodeNext"andmoduleResolution: "NodeNext".jsextensions to all relative importsSources
I find the tsconfig settings to be wildly confusing, but these seemed to indicate this was the right choice:
Consequences
.jsextensionsserver.deps.inline