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

chore(build): Avoid prebuilding api side, instead use an esbuild plugin #7672

Merged
merged 31 commits into from
Apr 5, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
096674f
Use async build
dac09 Feb 22, 2023
cd49f30
Fix tests
dac09 Feb 22, 2023
b3ece67
Remove graphql-tag plugin because it doesnt work anyway
dac09 Feb 22, 2023
7f1c721
Explain use of prebuildApiFiles
dac09 Feb 22, 2023
bd2c985
Rename prebuild function
dac09 Feb 22, 2023
9c8fbde
Remove redundant code
dac09 Feb 22, 2023
00a4a85
Revert "Remove graphql-tag plugin because it doesnt work anyway"
dac09 Feb 22, 2023
8932833
Fix failing nftPack test
dac09 Feb 23, 2023
480a257
Fix sourcemaps/debugging with breakpoints
dac09 Feb 23, 2023
3f84ed2
Fix api build test
dac09 Feb 23, 2023
d86822e
Try disabling telemetry on CLI tests
dac09 Feb 24, 2023
ccebbeb
Merge branch 'main' of github.com:redwoodjs/redwood into feat/api-ski…
dac09 Feb 24, 2023
f2ebf21
Try mocking telemetry
dac09 Feb 24, 2023
113ce60
Add "." to esbuild filter
dac09 Feb 24, 2023
dc03ee1
Remove prebuild
dac09 Feb 24, 2023
0f8f8b2
Remove unused import
dac09 Feb 24, 2023
4863387
Increase test timeout
dac09 Feb 24, 2023
c862986
Merge branch 'main' into feat/api-skip-prebuild
dac09 Mar 1, 2023
5d6557e
Mock FS in tests
Tobbe Mar 31, 2023
3308408
Merge branch 'main' into feat/api-skip-prebuild
Tobbe Mar 31, 2023
5b8fdd2
Fix test after merge
Tobbe Mar 31, 2023
fd575ba
Fix merge conflicts
Tobbe Mar 31, 2023
de923ae
Fix merge conflicts
Tobbe Mar 31, 2023
3010ad9
fix tests on windows
Tobbe Mar 31, 2023
18db1cd
Merge branch 'main' into feat/api-skip-prebuild
Tobbe Apr 1, 2023
4c1928b
Merge branch 'main' into feat/api-skip-prebuild
Tobbe Apr 2, 2023
333cac8
Undo jest config change
dac09 Apr 3, 2023
9bac85b
Undo telemetry change
dac09 Apr 3, 2023
10cb886
Merge branch 'main' into feat/api-skip-prebuild
dac09 Apr 4, 2023
96c455e
Merge branch 'main' into feat/api-skip-prebuild
thedavidprice Apr 4, 2023
c441253
Merge branch 'main' into feat/api-skip-prebuild
Tobbe Apr 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/api-server/src/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,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 = {
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/commands/buildHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/commands/deploy/__tests__/nftPack.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ const FIXTURE_PATH = path.resolve(

let functionDistFiles

beforeAll(() => {
beforeAll(async () => {
dac09 marked this conversation as resolved.
Show resolved Hide resolved
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()
await buildApi()
}

functionDistFiles = findApiDistFunctions()
Expand Down
28 changes: 27 additions & 1 deletion packages/internal/src/__tests__/build_api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import path from 'path'
import * as babel from '@babel/core'
import compat from 'core-js-compat'

import { cleanApiBuild, prebuildApiFiles } from '../build/api'
import { cleanApiBuild } from '../build/api'
import { transformWithBabel } from '../build/babel/api'
import {
getApiSideBabelConfigPath,
getApiSideBabelPlugins,
Expand All @@ -20,6 +21,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))
}
Expand Down
68 changes: 28 additions & 40 deletions packages/internal/src/build/api.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,19 @@
import fs from 'fs'
import path from 'path'

import * as esbuild from 'esbuild'
import { build } from 'esbuild'
import type { PluginBuild } from 'esbuild'
import { removeSync } from 'fs-extra'

import { findApiFiles } from '../files'
import { getPaths } from '../paths'

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 = () => {
Expand All @@ -29,44 +22,39 @@ export const cleanApiBuild = () => {
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) => {
jtoar marked this conversation as resolved.
Show resolved Hide resolved
// 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}`)
jtoar marked this conversation as resolved.
Show resolved Hide resolved
})
},
}

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
Expand Down
8 changes: 1 addition & 7 deletions packages/internal/src/build/babel/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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
Expand Down