Add support for developers to replace the default esbuild configuration

[dcousens]

This pull request addresses the problem in #8406 where the default esbuild configuration simply cannot account for how developers are integrating keystone-6 into their projects, and thus, rather than relying on patching a package, maybe we can provide an appropriate escape hatch.

Feedback welcome before merge

[acburdine]

this would definitely work for the use case I mentioned in #8406 - though I wonder if there isn't also a simpler way to solve certain use-cases without the need for an escape hatch. I've got another separate project that doesn't involve a monorepo per se - but it does use the tsconfig paths configuration pretty heavily to avoid ../../../../-type imports, and that project also has to have a similar workaround to ensure those aliased local files get included.

What we're doing in our workaround in both projects is effectively making the keystone esbuild options respect the typescript config's paths setting, and ensuring any paths listed there are also included by esbuild. Something like tsconfig-paths might be usable here - any paths in the list would be included.

The ability to completely customize the esbuild configuration would be useful, but it seems like it would introduce several new ways for devs to trip themselves up, especially if the default esbuild configuration were to be changed in any way in the future (i.e. if the output path/format/etc changed at all).

[acburdine]

did some experimenting locally in our monorepo with tsconfig-paths - this setup seemed to work fairly well:

// packages/core/src/lib/esbuild.ts
import { loadConfig } from "tsconfig-paths";
import { type BuildOptions } from 'esbuild'

function getPathsMatcher(cwd: string): RegExp | null {
  try {
    const pathsResult = loadConfig(cwd);
    if (pathsResult.resultType !== 'success') return null;

    const pathMatchers = Object.keys(tsConfigPaths.paths).map((path) => path.replace(/\*/, '.*'));
    if (!pathMatchers.length) return null;

    return new RegExp(`^(?:${pathMatchers.join('|')})$`);
  } catch {
    return null;
  }
}

export function getEsbuildConfig (cwd: string): BuildOptions {
  const pathMatcher = getPathMatcher(cwd);

  return {
    entryPoints: ['./keystone'],
    // ...rest of fields
    plugins: [
      {
        name: 'external-node_modules',
        setup (build) {
          build.onResolve(
            {
              // don't bundle anything that is NOT a relative import
              //   WARNING: we can't use a negative lookahead/lookbehind because esbuild uses Go
              filter: /(?:^[^.])|(?:^\.[^/.])|(?:^\.\.[^/])/,
            },
            ({ path }) => {
              if (pathMatcher?.test(path)) return;
              return { external: true, path }
            }
          )
        },
      },
    ],
  }
}

[gautamsi]

Does it help use path Aliases in custom admin pages as well?

tsconfig-paths may not work on custom admin pages as they are compiled in isolated context (.keystone/.admin). Once #9186 land it should work. I have tested that working without any esbuild changes.

if #9235 (comment) gets included, we may not even need the custom esbuild config, but we can still provide the support to load custom config

[acburdine]

Does it help use path Aliases in custom admin pages as well?

since admin pages are compliled directly with next.js, I think path aliases in admin pages should work out of the box already since Next.js supports it.

[marekryb]

The ability to completely customize the esbuild configuration would be useful, but it seems like it would introduce several new ways for devs to trip themselves up, especially if the default esbuild configuration were to be changed in any way in the future (i.e. if the output path/format/etc changed at all).

I tend to agree. However, apart from import paths, I would like to be able to change entryPoints (eg. src/keystone.ts instead of default) and potentially jsx or tsconfig too. So how about providing the defaults through function like:

// esbuild.config.js
export default function (defaultValues) {
  return {
    ...defaultValues,
    entryPoints: ['./src/keystone'],
  }
}

The tsconfig-paths thing seems great and can be added separately as an improvement. Shouldn't it be opt-in though? Just in case tsconfig.json has some strange things. Eg. would the common monorepo pattern "extends": "../../tsconfig.base.json", work?

[dcousens]

@marekryb that's a solid suggestion, I'll update esbuild.keystone.js to be a function that accepts a defaultValues

[dcousens]

@marekryb @acburdine @gautamsi I have added support based on @marekryb's suggestion, and additionally added support for Typescript, now you can use either of:

• esbuild.keystone.ts, or
• esbuild.keystone.js

This was easy to add, and I couldn't foresee any reason why we shouldn't.

[acburdine]

@dcousens i think this function approach makes sense - can always make an example showing how to implement something like the tsconfig-paths utility using the custom options file 🙂

[dcousens]

I'm happy for the scope of that example to be expanded on separately to this pull request, as the example already added shows how to use the functionality generally.

[asgeo1]

I'm trying to get absolute paths working with Keystone. I came across this PR, and was hopeful it would work.

I've tried setting up tsconfig-paths with esbuild.keystone.ts here:

// esbuild.keystone.ts

import { loadConfig } from 'tsconfig-paths'
import { type BuildOptions } from 'esbuild'

function getPathsMatcher(cwd?: string): RegExp | null {
  try {
    const pathsResult = loadConfig(cwd)
    if (pathsResult.resultType !== 'success') return null

    const pathMatchers = Object.keys(pathsResult.paths).map(path => path.replace(/\*/, '.*'))
    if (!pathMatchers.length) return null

    return new RegExp(`^(?:${pathMatchers.join('|')})$`)
  } catch {
    return null
  }
}

export default function (defaults: BuildOptions) {
  const pathMatcher = getPathsMatcher(defaults.absWorkingDir)
  return {
    ...defaults,
    plugins: [
      {
        name: 'external-node_modules',
        setup(build: any) {
          build.onResolve(
            {
              // don't bundle anything that is NOT a relative import
              //   WARNING: we can't use a negative lookahead/lookbehind because esbuild uses Go
              filter: /(?:^[^.])|(?:^\.[^/.])|(?:^\.\.[^/])/
            },
            ({ path }: { path: string }) => {
              if (pathMatcher?.test(path)) return
              return { external: true, path }
            }
          )
        }
      }
    ]
  }
}

It does work for most of my code, but not for the custom Keystone admin pages, which seems explained by @gautamsi 's comment above.

Given PR#9186 is not yet merged, is there any other known work around to get absolute paths working with custom Keystone admin pages?

[gautamsi]

@asgeo1 I added info about temp package while we wait for merging the app router part. I am already using with a large project which also includes heavily customized pages, all works good.