Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: give stable barrel file names #509

Merged
merged 2 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 14 additions & 13 deletions node/npm_dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,19 @@ import { Logger } from './logger.js'

const TYPESCRIPT_EXTENSIONS = new Set(['.ts', '.cts', '.mts'])

const slugifyPackageName = (specifier: string) => {
if (!specifier.startsWith('@')) return specifier
const [scope, pkg] = specifier.split('/')
return `${scope.replace('@', '')}__${pkg}`
}

/**
* Returns the name of the `@types/` package used by a given specifier. Most of
* the times this is just the specifier itself, but scoped packages suffer a
* transformation (e.g. `@netlify/functions` -> `@types/netlify__functions`).
* https://github.com/DefinitelyTyped/DefinitelyTyped#what-about-scoped-packages
*/
const getTypesPackageName = (specifier: string) => {
if (!specifier.startsWith('@')) return path.join('@types', specifier)
const [scope, pkg] = specifier.split('/')
return path.join('@types', `${scope.replace('@', '')}__${pkg}`)
}
const getTypesPackageName = (specifier: string) => path.join('@types', slugifyPackageName(specifier))

/**
* Finds corresponding DefinitelyTyped packages (`@types/...`) and returns path to declaration file.
Expand Down Expand Up @@ -233,24 +235,23 @@ export const vendorNPMSpecifiers = async ({
return
}

// To bundle an entire module and all its dependencies, create a barrel file
// To bundle an entire module and all its dependencies, create a entrypoint file
// where we re-export everything from that specifier. We do this for every
// specifier, and each of these files will become entry points to esbuild.
const ops = await Promise.all(
npmSpecifiers.map(async ({ specifier, types }, index) => {
npmSpecifiers.map(async ({ specifier, types }) => {
const code = `import * as mod from "${specifier}"; export default mod.default; export * from "${specifier}";`
const barrelName = `barrel-${index}.js`
const filePath = path.join(temporaryDirectory.path, barrelName)
const filePath = path.join(temporaryDirectory.path, `bundled-${slugifyPackageName(specifier)}.js`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why you went with the bundled- prefix here, rather than just using the slugified package name?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was worried that there might be an NPM package valid called chunk-1234, and that the name clashes with the chunk files that ESBuild generates :D


await fs.writeFile(filePath, code)

return { filePath, specifier, barrelName, types }
return { filePath, specifier, types }
}),
)
const entryPoints = ops.map(({ filePath }) => filePath)

// Bundle each of the barrel files we created. We'll end up with a compiled
// version of each of the barrel files, plus any chunks of shared code
// Bundle each of the entrypoints we created. We'll end up with a compiled
// version of each, plus any chunks of shared code
// between them (such that a common module isn't bundled twice).
const { outputFiles } = await build({
allowOverwrite: true,
Expand All @@ -269,7 +270,7 @@ export const vendorNPMSpecifiers = async ({

await Promise.all(
outputFiles.map(async (file) => {
const types = ops.find((op) => file.path.endsWith(op.barrelName))?.types
const types = ops.find((op) => path.basename(file.path) === path.basename(op.filePath))?.types
let content = file.text
if (types) {
content = `/// <reference types="${path.relative(file.path, types)}" />\n${content}`
Expand Down
4 changes: 2 additions & 2 deletions node/server/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,12 @@ test('Starts a server and serves requests for edge functions', async () => {
local: 'i love netlify',
})

const idBarrelFile = await readFile(join(servePath, 'barrel-0.js'), 'utf-8')
const idBarrelFile = await readFile(join(servePath, 'bundled-id.js'), 'utf-8')
expect(idBarrelFile).toContain(
`/// <reference types="${join('..', '..', '..', 'node_modules', 'id', 'types.d.ts')}" />`,
)

const identidadeBarrelFile = await readFile(join(servePath, 'barrel-2.js'), 'utf-8')
const identidadeBarrelFile = await readFile(join(servePath, 'bundled-pt-committee__identidade.js'), 'utf-8')
expect(identidadeBarrelFile).toContain(
`/// <reference types="${join(
'..',
Expand Down