Skip to content

Commit caa436e

Browse files
authored
fix(babel-config): dynamic import regression (#493)
1 parent 13cf36d commit caa436e

File tree

2 files changed

+88
-5
lines changed

2 files changed

+88
-5
lines changed
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
Supporting dynamic imports like `const mod = await import('src/lib/module.js')`
2+
in jobs.
3+
4+
With CedarJS v0.14.0 dynamic imports in jobs broke. Jobs are executed by a node
5+
process that's running jobs/src/bin/rw-jobs-worker.ts. rw-jobs-worker, if you
6+
follow the code deep enough, uses jobs/src/loaders.ts to `await import` the
7+
user-defined job and runs it. It loads from `getPaths().api.distJobs`, i.e. the
8+
built version of the job.
9+
So, basically, when running jobs we're just using plain Node, passing in a .js
10+
file. This tells me that it's our regular build process that's responsible for
11+
the bug that broke dynamic imports.
12+
13+
Cedar v0.14.0 was the same version that I released the work I had done on
14+
[api-side extensionless imports](./api-extensionless-imports.md). So I guessed
15+
it was related.
16+
17+
In `getApiSideBabelPlugins()` (as mentioned in that file above), we remove .js
18+
file extensions to let babel figure out what file to actually import. We can't
19+
do that for dynamic imports though, because they always need the file extension
20+
when Node is running the file.
21+
Keeping it around, however, breaks data migrations with an error like this:
22+
23+
```
24+
Error: Cannot find module './logger.js'
25+
Require stack:
26+
- /Users/tobbe/tmp/cedar-0140-jobs/api/src/lib/db.ts
27+
- /Users/tobbe/tmp/cedar-0140-jobs/node_modules/@cedarjs/cli-data-migrate/dist/commands/upHandler.js
28+
```
29+
30+
And the reason is that it's actually a .ts file.
31+
32+
Found an old comment in git history that confirms this
33+
34+
// If the .js file doesn't exist but the .ts file does, remove
35+
// the extension and let babel figure out what file to import.
36+
// Have to do this because I was having problems with imports
37+
// like `import { logger } from './logger.js'` in data-migrate
38+
// scripts in CJS projects
39+
40+
Here's a good example to test with, to make sure both regular and dynamic
41+
imports work:
42+
43+
```ts
44+
import type { PrismaClient } from '@prisma/client'
45+
46+
import { libLog } from './lib/lib.js'
47+
48+
export default async ({ db: _ }: { db: PrismaClient }) => {
49+
libLog('test')
50+
51+
const { libLog: asyncLibLog } = await import('./lib/lib.js')
52+
asyncLibLog('test')
53+
}
54+
```
55+
56+
For data-migrate we can actually treat regular and dynamic imports the same way
57+
and the reason for that is that data migrations are executed within the full
58+
Cedar CLI environment that has babel's require hook loaded. So, in contrast to
59+
jobs, which are using node to run built files, data migrations use babel to do
60+
the TS transformation and it also takes care of module resolutions, allowing it
61+
to run source files directly.
62+
63+
CedarJS currently have a few different places where code transformation happens:
64+
65+
- build
66+
- dev
67+
- data-migrate
68+
- jobs
69+
- exec
70+
- prerender
71+
72+
Only data-migrate and prerender uses babel to execute source files directly. And
73+
they only do so for CJS projects. As soon as we move to ESM only, all of this
74+
will be much simpler
75+
76+
prerender executes routeHooks, but it does so using `exec`, and that's not using
77+
babel's require hook (it's using vite-node). But prerender _also_ dynamically
78+
imports the user's graphql handler. And this is done with babel (in CJS
79+
projects). So we need to have the same behavior as for data-migrate.

packages/babel-config/src/api.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -108,11 +108,15 @@ export const getApiSideBabelPlugins = ({
108108
currentFile: string,
109109
opts: unknown,
110110
) {
111-
// Look for paths that include a / and ends with .js (to not trip
112-
// up on npm modules like chart.js)
113-
const importPath = /.*\/.*\.js$/.test(sourcePath)
114-
? sourcePath.replace(/\.js$/, '')
115-
: sourcePath
111+
// To support imports like `import { logger } from './logger.js'` in
112+
// data-migrate and prerender in TypeScript projects (where the actual
113+
// source file is logger.ts) we have to rewrite the extension
114+
const isDataMigrate = process.argv[2] === 'data-migrate'
115+
const isPrerender = process.argv[2] === 'prerender'
116+
const importPath =
117+
/.*\/.*\.js$/.test(sourcePath) && (isDataMigrate || isPrerender)
118+
? sourcePath.replace(/\.js$/, '')
119+
: sourcePath
116120

117121
const resolvedPath = resolvePath(importPath, currentFile, opts)
118122

0 commit comments

Comments
 (0)