From af7a998d21c3cef271b4c745e7e611541a47b5f6 Mon Sep 17 00:00:00 2001 From: Daniel Choudhury Date: Wed, 5 Apr 2023 13:49:20 +0700 Subject: [PATCH] chore(build): Avoid prebuilding api side, instead use an esbuild plugin (#7672) Co-authored-by: Tobbe Lundberg Co-authored-by: David Price --- packages/api-server/src/watch.ts | 4 +- packages/cli/jsconfig.json | 3 +- .../cli/src/commands/__tests__/build.test.js | 4 +- packages/cli/src/commands/buildHandler.js | 7 +- .../commands/deploy/__tests__/nftPack.test.js | 47 ++++++------ .../internal/src/__tests__/build_api.test.ts | 37 +++++++++- packages/internal/src/build/api.ts | 71 ++++++++----------- packages/internal/src/build/babel/api.ts | 8 +-- 8 files changed, 98 insertions(+), 83 deletions(-) diff --git a/packages/api-server/src/watch.ts b/packages/api-server/src/watch.ts index 756b355cb776..0be01404f60c 100644 --- a/packages/api-server/src/watch.ts +++ b/packages/api-server/src/watch.ts @@ -61,14 +61,14 @@ const validate = async () => { } } -const rebuildApiServer = () => { +const rebuildApiServer = async () => { try { // Shutdown API server killApiServer() const buildTs = Date.now() process.stdout.write(c.dim(c.italic('Building... '))) - buildApi() + await buildApi() console.log(c.dim(c.italic('Took ' + (Date.now() - buildTs) + ' ms'))) const forkOpts = { diff --git a/packages/cli/jsconfig.json b/packages/cli/jsconfig.json index 0b2607fe6cfc..1b8d45ca0773 100644 --- a/packages/cli/jsconfig.json +++ b/packages/cli/jsconfig.json @@ -12,7 +12,6 @@ { "path": "../project-config" }, { "path": "../structure" }, { "path": "../prerender" }, - { "path": "../api-server" }, - { "path": "../auth-providers-setup" }, + { "path": "../api-server" } ] } diff --git a/packages/cli/src/commands/__tests__/build.test.js b/packages/cli/src/commands/__tests__/build.test.js index b54468d9d483..2a9906caa2bc 100644 --- a/packages/cli/src/commands/__tests__/build.test.js +++ b/packages/cli/src/commands/__tests__/build.test.js @@ -67,7 +67,7 @@ test('Should run prerender for web', async () => { // Run prerendering task, but expect warning, // because `detectPrerenderRoutes` is empty. expect(consoleSpy.mock.calls[0][0]).toBe('Starting prerendering...') - expect(consoleSpy.mock.calls[1][0]).toBe( - 'You have not marked any routes to "prerender" in your Routes (​file:///mocked/project/web/Routes.tsx​).' + expect(consoleSpy.mock.calls[1][0]).toMatch( + /You have not marked any routes to "prerender"/ ) }) diff --git a/packages/cli/src/commands/buildHandler.js b/packages/cli/src/commands/buildHandler.js index 841fe51c0d7d..bf891e805edb 100644 --- a/packages/cli/src/commands/buildHandler.js +++ b/packages/cli/src/commands/buildHandler.js @@ -73,8 +73,8 @@ export const handler = async ({ }, side.includes('api') && { title: 'Building API...', - task: () => { - const { errors, warnings } = buildApi() + task: async () => { + const { errors, warnings } = await buildApi() if (errors.length) { console.error(errors) @@ -134,7 +134,10 @@ export const handler = async ({ 'file://' + rwjsPaths.web.routes )}.` ) + + return } + // Running a separate process here, otherwise it wouldn't pick up the // generated Prisma Client due to require module caching await execa('yarn rw prerender', { diff --git a/packages/cli/src/commands/deploy/__tests__/nftPack.test.js b/packages/cli/src/commands/deploy/__tests__/nftPack.test.js index a3f79a6e8572..62e859a49c6a 100644 --- a/packages/cli/src/commands/deploy/__tests__/nftPack.test.js +++ b/packages/cli/src/commands/deploy/__tests__/nftPack.test.js @@ -1,31 +1,32 @@ -import fs from 'fs' -import path from 'path' - -import { buildApi } from '@redwoodjs/internal/dist/build/api' import { findApiDistFunctions } from '@redwoodjs/internal/dist/files' import nftPacker from '../packing/nft' -const FIXTURE_PATH = path.resolve( - __dirname, - '../../../../../../__fixtures__/example-todo-main' -) - -let functionDistFiles - -beforeAll(() => { - process.env.RWJS_CWD = FIXTURE_PATH - - // Actually build the fixture, if we need it - if (!fs.existsSync(path.join(FIXTURE_PATH, 'api/dist/functions'))) { - buildApi() +jest.mock('@redwoodjs/internal/dist/files', () => { + return { + findApiDistFunctions: () => { + return [ + '/Users/carmack/dev/redwood/__fixtures__/example-todo-main/api/dist/functions/graphql.js', + '/Users/carmack/dev/redwood/__fixtures__/example-todo-main/api/dist/functions/healthz/healthz.js', + '/Users/carmack/dev/redwood/__fixtures__/example-todo-main/api/dist/functions/invalid/x.js', + '/Users/carmack/dev/redwood/__fixtures__/example-todo-main/api/dist/functions/nested/nested.js', + '/Users/carmack/dev/redwood/__fixtures__/example-todo-main/api/dist/functions/x/index.js', + ] + }, } - - functionDistFiles = findApiDistFunctions() }) -afterAll(() => { - delete process.env.RWJS_CWD +jest.mock('@redwoodjs/project-config', () => { + return { + getPaths: () => { + return { + base: '/Users/carmack/dev/redwood/__fixtures__/example-todo-main/', + } + }, + ensurePosixPath: (path) => { + return path.replace(/\\/g, '/') + }, + } }) test('Check packager detects all functions', () => { @@ -39,7 +40,7 @@ test('Check packager detects all functions', () => { }) test('Creates entry file for nested functions correctly', () => { - const nestedFunction = functionDistFiles.find((fPath) => + const nestedFunction = findApiDistFunctions().find((fPath) => fPath.includes('nested') ) @@ -55,7 +56,7 @@ test('Creates entry file for nested functions correctly', () => { }) test('Creates entry file for top level functions correctly', () => { - const graphqlFunction = functionDistFiles.find((fPath) => + const graphqlFunction = findApiDistFunctions().find((fPath) => fPath.includes('graphql') ) diff --git a/packages/internal/src/__tests__/build_api.test.ts b/packages/internal/src/__tests__/build_api.test.ts index c46bb09a9e82..5c90d9965f4a 100644 --- a/packages/internal/src/__tests__/build_api.test.ts +++ b/packages/internal/src/__tests__/build_api.test.ts @@ -6,11 +6,12 @@ import compat from 'core-js-compat' import { ensurePosixPath, getPaths } from '@redwoodjs/project-config' -import { cleanApiBuild, prebuildApiFiles } from '../build/api' +import { cleanApiBuild } from '../build/api' import { getApiSideBabelConfigPath, getApiSideBabelPlugins, getApiSideDefaultBabelConfig, + transformWithBabel, BABEL_PLUGIN_TRANSFORM_RUNTIME_OPTIONS, TARGETS_NODE, } from '../build/babel/api' @@ -21,6 +22,31 @@ const FIXTURE_PATH = path.resolve( '../../../../__fixtures__/example-todo-main' ) +// @NOTE: we no longer prebuild files into the .redwood/prebuild folder +// However, prebuilding in the tests still helpful for us to validate +// that everything is working as expected. +export const prebuildApiFiles = (srcFiles: string[]) => { + const rwjsPaths = getPaths() + const plugins = getApiSideBabelPlugins() + + return srcFiles.map((srcPath) => { + const relativePathFromSrc = path.relative(rwjsPaths.base, srcPath) + const dstPath = path + .join(rwjsPaths.generated.prebuild, relativePathFromSrc) + .replace(/\.(ts)$/, '.js') + + const result = transformWithBabel(srcPath, plugins) + if (!result?.code) { + throw new Error(`Could not prebuild ${srcPath}`) + } + + fs.mkdirSync(path.dirname(dstPath), { recursive: true }) + fs.writeFileSync(dstPath, result.code) + + return dstPath + }) +} + const cleanPaths = (p) => { return ensurePosixPath(path.relative(FIXTURE_PATH, p)) } @@ -90,6 +116,10 @@ test.skip('api prebuild transforms gql with `babel-plugin-graphql-tag`', () => { .filter((p) => p.endsWith('todos.sdl.js')) .pop() + if (!p) { + throw new Error('No built files') + } + const code = fs.readFileSync(p, 'utf-8') expect(code.includes('import gql from "graphql-tag";')).toEqual(false) expect(code.includes('gql`')).toEqual(false) @@ -493,7 +523,7 @@ test('jest mock statements also handle', () => { cwd: getPaths().api.base, // We override the plugins, to match packages/testing/config/jest/api/index.js plugins: getApiSideBabelPlugins({ forJest: true }), - }).code + })?.code // Step 2: check that output has correct import statement path expect(outputForJest).toContain('import dog from "../../lib/dog"') @@ -503,10 +533,13 @@ test('jest mock statements also handle', () => { test('core-js polyfill list', () => { const { list } = compat({ + // @ts-expect-error TODO: Figure out why this is needed targets: { node: TARGETS_NODE }, + // @ts-expect-error TODO: Figure out why this is needed version: BABEL_PLUGIN_TRANSFORM_RUNTIME_OPTIONS.corejs.version, }) + // TODO: Node.js 12? Really? /** * Redwood targets Node.js 12, but that doesn't factor into what gets polyfilled * because Redwood uses the plugin-transform-runtime polyfill strategy. diff --git a/packages/internal/src/build/api.ts b/packages/internal/src/build/api.ts index efa1e17024ab..ac2bb71e366c 100644 --- a/packages/internal/src/build/api.ts +++ b/packages/internal/src/build/api.ts @@ -1,73 +1,58 @@ -import fs from 'fs' -import path from 'path' - -import * as esbuild from 'esbuild' +import type { PluginBuild } from 'esbuild' +import { build } from 'esbuild' import { removeSync } from 'fs-extra' import { getPaths } from '@redwoodjs/project-config' import { findApiFiles } from '../files' -import { getApiSideBabelPlugins, prebuildApiFile } from './babel/api' +import { getApiSideBabelPlugins, transformWithBabel } from './babel/api' -export const buildApi = () => { +export const buildApi = async () => { // TODO: Be smarter about caching and invalidating files, // but right now we just delete everything. cleanApiBuild() - - const srcFiles = findApiFiles() - - const prebuiltFiles = prebuildApiFiles(srcFiles).filter( - (path): path is string => path !== undefined - ) - - return transpileApi(prebuiltFiles) + return transpileApi(findApiFiles()) } export const cleanApiBuild = () => { const rwjsPaths = getPaths() removeSync(rwjsPaths.api.dist) - removeSync(path.join(rwjsPaths.generated.prebuild, 'api')) } -/** - * Remove RedwoodJS "magic" from a user's code leaving JavaScript behind. - */ -export const prebuildApiFiles = (srcFiles: string[]) => { - const rwjsPaths = getPaths() - const plugins = getApiSideBabelPlugins() - - return srcFiles.map((srcPath) => { - const relativePathFromSrc = path.relative(rwjsPaths.base, srcPath) - const dstPath = path - .join(rwjsPaths.generated.prebuild, relativePathFromSrc) - .replace(/\.(ts)$/, '.js') - - const result = prebuildApiFile(srcPath, dstPath, plugins) - if (!result?.code) { - // TODO: Figure out a better way to return these programatically. - console.warn('Error:', srcPath, 'could not prebuilt.') - - return undefined - } - - fs.mkdirSync(path.dirname(dstPath), { recursive: true }) - fs.writeFileSync(dstPath, result.code) - - return dstPath - }) +const runRwBabelTransformsPlugin = { + name: 'rw-esbuild-babel-transform', + setup(build: PluginBuild) { + build.onLoad({ filter: /\.(js|ts|tsx|jsx)$/ }, async (args) => { + // Remove RedwoodJS "magic" from a user's code leaving JavaScript behind. + const transformedCode = transformWithBabel( + args.path, + getApiSideBabelPlugins() + ) + + if (transformedCode?.code) { + return { + contents: transformedCode.code, + loader: 'js', + } + } + + throw new Error(`Could not transform file: ${args.path}`) + }) + }, } -export const transpileApi = (files: string[], options = {}) => { +export const transpileApi = async (files: string[], options = {}) => { const rwjsPaths = getPaths() - return esbuild.buildSync({ + return build({ absWorkingDir: rwjsPaths.api.base, entryPoints: files, platform: 'node', target: 'node16', format: 'cjs', bundle: false, + plugins: [runRwBabelTransformsPlugin], outdir: rwjsPaths.api.dist, // setting this to 'true' will generate an external sourcemap x.js.map // AND set the sourceMappingURL comment diff --git a/packages/internal/src/build/babel/api.ts b/packages/internal/src/build/babel/api.ts index ab6300cc7260..a44a13eaa4ad 100644 --- a/packages/internal/src/build/babel/api.ts +++ b/packages/internal/src/build/babel/api.ts @@ -194,11 +194,8 @@ export const registerApiSideBabelHook = ({ }) } -export const prebuildApiFile = ( +export const transformWithBabel = ( srcPath: string, - // we need to know dstPath as well - // so we can generate an inline, relative sourcemap - dstPath: string, plugins: TransformOptions['plugins'] ) => { const code = fs.readFileSync(srcPath, 'utf-8') @@ -208,9 +205,6 @@ export const prebuildApiFile = ( ...defaultOptions, cwd: getPaths().api.base, filename: srcPath, - // we set the sourceFile (for the sourcemap) as a correct, relative path - // this is why this function (prebuildFile) must know about the dstPath - sourceFileName: path.relative(path.dirname(dstPath), srcPath), // we need inline sourcemaps at this level // because this file will eventually be fed to esbuild // when esbuild finds an inline sourcemap, it tries to "combine" it