fix: ES module compatibility for migrations URL resolution#3373
fix: ES module compatibility for migrations URL resolution#3373mondoreale merged 6 commits intomainfrom
Conversation
This gets converted into env-specific resolution outside of esm (rollup does it).
Create `importMetaTransformer.js` that wraps `ts-jest` and replaces
`import.meta.url` with `file://${__filename}` before transformation,
enabling ESM-style URL references to work in Jest's CommonJS environment.
mondoreale
changed the title
__dirname in browser by using import.meta.url
There was a problem hiding this comment.
Pull request overview
This PR modernizes the PersistenceManager to use ES module syntax (import.meta.url) instead of CommonJS-specific globals (__dirname). The change improves compatibility with ES module environments and standardizes the codebase on ES module patterns. A custom Jest transformer was added to handle the transformation of import.meta.url to a CommonJS-compatible equivalent during test execution.
Changes:
- Replaced
file://${__dirname}/withimport.meta.urlin PersistenceManager for constructing the migrations URL - Added a custom Jest transformer that converts
import.meta.urlto a template string using__filenamefor test compatibility - Updated Jest configuration to use the custom transformer for TypeScript files
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/sdk/src/PersistenceManager.ts | Updated migrations URL construction to use import.meta.url instead of __dirname |
| packages/sdk/test/test-utils/importMetaTransformer.js | Added custom Jest transformer to handle import.meta.url in test environment |
| packages/sdk/jest.config.ts | Configured Jest to use the custom transformer for TypeScript files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ownerId: await this.identity.getUserId(), | ||
| namespaces: Object.values(NAMESPACES), | ||
| migrationsUrl: new URL('./encryption/migrations', `file://${__dirname}/`), | ||
| migrationsUrl: new URL('./encryption/migrations', import.meta.url), |
There was a problem hiding this comment.
The PR title mentions "browser compatibility" but PersistenceManager imports from '@/Persistence' which resolves to the Node.js-specific implementation. This code doesn't actually run in browsers - the browser version uses IndexedDB and doesn't use migrations. Consider updating the PR title/description to clarify that this change is about ES module compatibility or module format compatibility, not browser compatibility.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
mondoreale
changed the title
__dirname in browser by using import.meta.url
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Summary
Fix
undefined__dirnameerror in browser environments by usingimport.meta.urlfor migration URL resolution inPersistenceManager. Rollup handles the conversion to environment-specific resolution for non-ESM builds.Changes
import.meta.urlinstead of__dirnameinPersistenceManager.tsfor resolving migration pathsts-jestto replaceimport.meta.urlwithfile://${__filename}before transformation, enabling tests to run in Jest's CommonJS environmentNote
Ensures migrations path resolution works in ESM and Jest environments.
__dirnamewithimport.meta.urlformigrationsUrlinPersistenceManager.ts(withwebpackIgnore)test/test-utils/importMetaTransformer.mjsto rewriteimport.meta.urltofile://${__filename}forts-jestjest.config.tsto use the custom transformer for TypeScript filesWritten by Cursor Bugbot for commit b5e61d2. This will update automatically on new commits. Configure here.