Skip to content

Commit

Permalink
fix: support import maps in npm module resolution (netlify/edge-bundl…
Browse files Browse the repository at this point in the history
…er#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
  • Loading branch information
eduardoboucas authored Sep 7, 2023
1 parent 91847c7 commit ad78051
Show file tree
Hide file tree
Showing 9 changed files with 215 additions and 59 deletions.
1 change: 1 addition & 0 deletions packages/edge-bundler/node/bundler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
21 changes: 16 additions & 5 deletions packages/edge-bundler/node/bundler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -239,6 +246,7 @@ interface VendorNPMOptions {
basePath: string
featureFlags: FeatureFlags
functions: EdgeFunction[]
importMap: ImportMap
logger: Logger
vendorDirectory: string | undefined
}
Expand All @@ -247,6 +255,7 @@ const safelyVendorNPMSpecifiers = async ({
basePath,
featureFlags,
functions,
importMap,
logger,
vendorDirectory,
}: VendorNPMOptions) => {
Expand All @@ -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)
}
Expand Down
33 changes: 24 additions & 9 deletions packages/edge-bundler/node/import_map.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand Down
32 changes: 32 additions & 0 deletions packages/edge-bundler/node/import_map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down
Loading

0 comments on commit ad78051

Please sign in to comment.