fix: support import maps in npm module resolution (#471)

* fix: support import maps in npm module resolution * fix: set default value of flag * refactor: add user-facing messages * fix: fix Windows paths * fix: fix capitalisation
netlify · Sep 7, 2023 · 3f4975e · 3f4975e
1 parent 3d8b3f3
commit 3f4975e
Show file tree

Hide file tree

Showing 9 changed files with 215 additions and 59 deletions.
diff --git a/node/bundler.test.ts b/node/bundler.test.ts
@@ -472,6 +472,7 @@ test('Loads npm modules from bare specifiers with and without the `npm:` prefix'
   await bundle([sourceDirectory], distPath, declarations, {
     basePath,
     featureFlags: { edge_functions_npm_modules: true },
+    importMapPaths: [join(basePath, 'import_map.json')],
     vendorDirectory: vendorDirectory.path,
   })
   const manifestFile = await readFile(resolve(distPath, 'manifest.json'), 'utf8')

diff --git a/node/bundler.ts b/node/bundler.ts
@@ -97,7 +97,14 @@ export const bundle = async (
   const userFunctions = userSourceDirectories.length === 0 ? [] : await findFunctions(userSourceDirectories)
   const internalFunctions = internalSrcFolder ? await findFunctions([internalSrcFolder]) : []
   const functions = [...internalFunctions, ...userFunctions]
-  const vendor = await safelyVendorNPMSpecifiers({ basePath, featureFlags, functions, logger, vendorDirectory })
+  const vendor = await safelyVendorNPMSpecifiers({
+    basePath,
+    featureFlags,
+    functions,
+    importMap,
+    logger,
+    vendorDirectory,
+  })
 
   if (vendor) {
     importMap.add(vendor.importMap)
@@ -239,6 +246,7 @@ interface VendorNPMOptions {
   basePath: string
   featureFlags: FeatureFlags
   functions: EdgeFunction[]
+  importMap: ImportMap
   logger: Logger
   vendorDirectory: string | undefined
 }
@@ -247,6 +255,7 @@ const safelyVendorNPMSpecifiers = async ({
   basePath,
   featureFlags,
   functions,
+  importMap,
   logger,
   vendorDirectory,
 }: VendorNPMOptions) => {
@@ -255,11 +264,13 @@ const safelyVendorNPMSpecifiers = async ({
   }
 
   try {
-    return await vendorNPMSpecifiers(
+    return await vendorNPMSpecifiers({
       basePath,
-      functions.map(({ path }) => path),
-      vendorDirectory,
-    )
+      directory: vendorDirectory,
+      functions: functions.map(({ path }) => path),
+      importMap,
+      logger,
+    })
   } catch (error) {
     logger.system(error)
   }

diff --git a/node/import_map.test.ts b/node/import_map.test.ts
@@ -24,12 +24,20 @@ test('Handles import maps with full URLs without specifying a base URL', () => {
   }
 
   const map = new ImportMap([inputFile1, inputFile2])
-  const { imports } = map.getContents()
 
-  expect(imports['netlify:edge']).toBe('https://edge.netlify.com/v1/index.ts?v=legacy')
-  expect(imports['@netlify/edge-functions']).toBe('https://edge.netlify.com/v1/index.ts')
-  expect(imports['alias:jamstack']).toBe('https://jamstack.org/')
-  expect(imports['alias:pets']).toBe('https://petsofnetlify.com/')
+  const m1 = map.getContents()
+
+  expect(m1.imports['netlify:edge']).toBe('https://edge.netlify.com/v1/index.ts?v=legacy')
+  expect(m1.imports['@netlify/edge-functions']).toBe('https://edge.netlify.com/v1/index.ts')
+  expect(m1.imports['alias:jamstack']).toBe('https://jamstack.org/')
+  expect(m1.imports['alias:pets']).toBe('https://petsofnetlify.com/')
+
+  const m2 = map.getContentsWithURLObjects()
+
+  expect(m2.imports['netlify:edge']).toStrictEqual(new URL('https://edge.netlify.com/v1/index.ts?v=legacy'))
+  expect(m2.imports['@netlify/edge-functions']).toStrictEqual(new URL('https://edge.netlify.com/v1/index.ts'))
+  expect(m2.imports['alias:jamstack']).toStrictEqual(new URL('https://jamstack.org/'))
+  expect(m2.imports['alias:pets']).toStrictEqual(new URL('https://petsofnetlify.com/'))
 })
 
 test('Resolves relative paths to absolute paths if a base path is not provided', () => {
@@ -42,12 +50,19 @@ test('Resolves relative paths to absolute paths if a base path is not provided',
   }
 
   const map = new ImportMap([inputFile1])
-  const { imports } = map.getContents()
   const expectedPath = join(cwd(), 'my-cool-site', 'heart', 'pets')
 
-  expect(imports['netlify:edge']).toBe('https://edge.netlify.com/v1/index.ts?v=legacy')
-  expect(imports['@netlify/edge-functions']).toBe('https://edge.netlify.com/v1/index.ts')
-  expect(imports['alias:pets']).toBe(`${pathToFileURL(expectedPath).toString()}/`)
+  const m1 = map.getContents()
+
+  expect(m1.imports['netlify:edge']).toBe('https://edge.netlify.com/v1/index.ts?v=legacy')
+  expect(m1.imports['@netlify/edge-functions']).toBe('https://edge.netlify.com/v1/index.ts')
+  expect(m1.imports['alias:pets']).toBe(`${pathToFileURL(expectedPath).toString()}/`)
+
+  const m2 = map.getContentsWithURLObjects()
+
+  expect(m2.imports['netlify:edge']).toStrictEqual(new URL('https://edge.netlify.com/v1/index.ts?v=legacy'))
+  expect(m2.imports['@netlify/edge-functions']).toStrictEqual(new URL('https://edge.netlify.com/v1/index.ts'))
+  expect(m2.imports['alias:pets']).toStrictEqual(new URL(`${pathToFileURL(expectedPath).toString()}/`))
 })
 
 describe('Returns the fully resolved import map', () => {

diff --git a/node/import_map.ts b/node/import_map.ts
@@ -76,6 +76,26 @@ export class ImportMap {
     )
   }
 
+  static convertImportsToURLObjects(imports: Imports) {
+    return Object.entries(imports).reduce(
+      (acc, [key, value]) => ({
+        ...acc,
+        [key]: new URL(value),
+      }),
+      {} as Record<string, URL>,
+    )
+  }
+
+  static convertScopesToURLObjects(scopes: Record<string, Imports>) {
+    return Object.entries(scopes).reduce(
+      (acc, [key, value]) => ({
+        ...acc,
+        [key]: ImportMap.convertImportsToURLObjects(value),
+      }),
+      {} as Record<string, Record<string, URL>>,
+    )
+  }
+
   // Applies a list of prefixes to a given path, returning the replaced path.
   // For example, given a `path` of `file:///foo/bar/baz.js` and a `prefixes`
   // object with `{"file:///foo/": "file:///hello/"}`, this method will return
@@ -180,6 +200,18 @@ export class ImportMap {
     }
   }
 
+  // The same as `getContents`, but the URLs are represented as URL objects
+  // instead of strings. This is compatible with the `ParsedImportMap` type
+  // from the `@import-maps/resolve` library.
+  getContentsWithURLObjects(prefixes: Record<string, string> = {}) {
+    const { imports, scopes } = this.getContents(prefixes)
+
+    return {
+      imports: ImportMap.convertImportsToURLObjects(imports),
+      scopes: ImportMap.convertScopesToURLObjects(scopes),
+    }
+  }
+
   static async readFile(path: string, logger: Logger): Promise<ImportMapFile> {
     const baseURL = pathToFileURL(path)