Skip to content

Commit bbe9313

Browse files
authored
fix(edge-bundler): only parse a file once for it's npm specifiers (#6196)
1 parent 088efc4 commit bbe9313

File tree

5 files changed

+70
-4
lines changed

5 files changed

+70
-4
lines changed

packages/edge-bundler/node/bundler.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,33 @@ test('Loads npm modules which use package.json.exports', async () => {
533533
await rm(vendorDirectory.path, { force: true, recursive: true })
534534
})
535535

536+
test('Loads modules which contain cycles', async () => {
537+
const { basePath, cleanup, distPath } = await useFixture('imports_cycle')
538+
const sourceDirectory = join(basePath, 'functions')
539+
const declarations: Declaration[] = [
540+
{
541+
function: 'func1',
542+
path: '/func1',
543+
},
544+
]
545+
const vendorDirectory = await tmp.dir()
546+
547+
await bundle([sourceDirectory], distPath, declarations, {
548+
basePath,
549+
vendorDirectory: vendorDirectory.path,
550+
})
551+
552+
const manifestFile = await readFile(resolve(distPath, 'manifest.json'), 'utf8')
553+
const manifest = JSON.parse(manifestFile)
554+
const bundlePath = join(distPath, manifest.bundles[0].asset)
555+
const { func1 } = await runESZIP(bundlePath, vendorDirectory.path)
556+
557+
expect(func1).toBe('magix')
558+
559+
await cleanup()
560+
await rm(vendorDirectory.path, { force: true, recursive: true })
561+
})
562+
536563
test('Loads npm modules in a monorepo setup', async () => {
537564
const systemLogger = vi.fn()
538565
const { basePath: rootPath, cleanup, distPath } = await useFixture('monorepo_npm_module')

packages/edge-bundler/node/npm_dependencies.ts

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,10 @@ async function parseImportsForFile(file: string, rootPath: string) {
145145
* Parses a set of functions and returns a list of specifiers that correspond
146146
* to npm modules.
147147
*/
148-
const getNPMSpecifiers = async ({ basePath, functions, importMap, environment, rootPath }: GetNPMSpecifiersOptions) => {
148+
const getNPMSpecifiers = async (
149+
{ basePath, functions, importMap, environment, rootPath }: GetNPMSpecifiersOptions,
150+
alreadySeenPaths = new Set<string>(),
151+
) => {
149152
const baseURL = pathToFileURL(basePath)
150153
const npmSpecifiers: { specifier: string; types?: string }[] = []
151154
for (const func of functions) {
@@ -157,15 +160,29 @@ const getNPMSpecifiers = async ({ basePath, functions, importMap, environment, r
157160
const specifier = i.moduleSpecifier.isConstant ? i.moduleSpecifier.value! : i.moduleSpecifier.code
158161
switch (i.moduleSpecifier.type) {
159162
case 'absolute': {
163+
if (alreadySeenPaths.has(specifier)) {
164+
break
165+
}
166+
alreadySeenPaths.add(specifier)
160167
npmSpecifiers.push(
161-
...(await getNPMSpecifiers({ basePath, functions: [specifier], importMap, environment, rootPath })),
168+
...(await getNPMSpecifiers(
169+
{ basePath, functions: [specifier], importMap, environment, rootPath },
170+
alreadySeenPaths,
171+
)),
162172
)
163173
break
164174
}
165175
case 'relative': {
166176
const filePath = path.join(path.dirname(func), specifier)
177+
if (alreadySeenPaths.has(filePath)) {
178+
break
179+
}
180+
alreadySeenPaths.add(filePath)
167181
npmSpecifiers.push(
168-
...(await getNPMSpecifiers({ basePath, functions: [filePath], importMap, environment, rootPath })),
182+
...(await getNPMSpecifiers(
183+
{ basePath, functions: [filePath], importMap, environment, rootPath },
184+
alreadySeenPaths,
185+
)),
169186
)
170187
break
171188
}
@@ -180,8 +197,15 @@ const getNPMSpecifiers = async ({ basePath, functions, importMap, environment, r
180197
if (matched) {
181198
if (resolvedImport.protocol === 'file:') {
182199
const newSpecifier = fileURLToPath(resolvedImport).replace(/\\/g, '/')
200+
if (alreadySeenPaths.has(newSpecifier)) {
201+
break
202+
}
203+
alreadySeenPaths.add(newSpecifier)
183204
npmSpecifiers.push(
184-
...(await getNPMSpecifiers({ basePath, functions: [newSpecifier], importMap, environment, rootPath })),
205+
...(await getNPMSpecifiers(
206+
{ basePath, functions: [newSpecifier], importMap, environment, rootPath },
207+
alreadySeenPaths,
208+
)),
185209
)
186210
}
187211
} else if (!resolvedImport?.protocol?.startsWith('http')) {
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import hello from '../hello.ts'
2+
3+
export const name = "magix"
4+
5+
export default async () => {
6+
return new Response(hello())
7+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { name } from "./functions/func1.ts";
2+
3+
export default function hello() {
4+
return name
5+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"type": "module"
3+
}

0 commit comments

Comments
 (0)