Skip to content

Commit

Permalink
fix: parse TSX files for module detection, define NODE_ENV, polyfill …
Browse files Browse the repository at this point in the history
…missing Node.js globals (netlify/edge-bundler#519)

* fix: parse TSX files for module detection, define NODE_ENV

* chore: remove comment

* fix: only define `process.env.NODE_ENV` for builds

* fix: implement polyfills for Node globals

* fix: remove .only

* refactor: use banner instead

* fix: ensure customer code can't access process.env

* fix: remove .only once again

* chore: cleanup foo var

* chore: align formatting

* refactor: replace two bools with environment
  • Loading branch information
Skn0tt authored Nov 14, 2023
1 parent 35b0145 commit 612776e
Show file tree
Hide file tree
Showing 10 changed files with 78 additions and 17 deletions.
2 changes: 1 addition & 1 deletion packages/edge-bundler/.eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { overrides } = require('@netlify/eslint-config-node')

module.exports = {
extends: '@netlify/eslint-config-node',
ignorePatterns: ['deno/**/*.ts', 'test/deno/**/*.ts', 'test/fixtures/**/*.ts'],
ignorePatterns: ['deno/**/*', 'test/deno/**/*', 'test/fixtures/**/*'],
parserOptions: {
sourceType: 'module',
},
Expand Down
32 changes: 32 additions & 0 deletions packages/edge-bundler/node/bundler.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Buffer } from 'buffer'
import { access, readdir, readFile, rm, writeFile } from 'fs/promises'
import { join, resolve } from 'path'
import process from 'process'
Expand Down Expand Up @@ -536,3 +537,34 @@ test('Loads JSON modules', async () => {
await cleanup()
await rm(vendorDirectory.path, { force: true, recursive: true })
})

test('Supports TSX and process.env', async () => {
const { basePath, cleanup, distPath } = await useFixture('tsx')
const sourceDirectory = join(basePath, 'functions')
const declarations: Declaration[] = [
{
function: 'func1',
path: '/func1',
},
]
const vendorDirectory = await tmp.dir()

await bundle([sourceDirectory], distPath, declarations, {
basePath,
vendorDirectory: vendorDirectory.path,
})

const manifestFile = await readFile(resolve(distPath, 'manifest.json'), 'utf8')
const manifest = JSON.parse(manifestFile)
const bundlePath = join(distPath, manifest.bundles[0].asset)
process.env.FOO = 'bar'
const { func1 } = await runESZIP(bundlePath, vendorDirectory.path)

expect(Buffer.from(func1, 'base64').toString()).toBe(
`hippedy hoppedy, createElement is now a production property. Here, take this env var: FOO=bar`,
)

await cleanup()
await rm(vendorDirectory.path, { force: true, recursive: true })
delete process.env.FOO
})
2 changes: 1 addition & 1 deletion packages/edge-bundler/node/bundler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ const safelyVendorNPMSpecifiers = async ({
functions: functions.map(({ path }) => path),
importMap,
logger,
referenceTypes: false,
environment: 'production',
rootPath,
})
} catch (error) {
Expand Down
31 changes: 17 additions & 14 deletions packages/edge-bundler/node/npm_dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { ImportMap } from './import_map.js'
import { Logger } from './logger.js'
import { pathsBetween } from './utils/fs.js'

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

const slugifyPackageName = (specifier: string) => {
if (!specifier.startsWith('@')) return specifier
Expand Down Expand Up @@ -81,6 +81,10 @@ const safelyDetectTypes = async (packageJsonPath: string): Promise<string | unde
// Workaround for https://github.com/evanw/esbuild/issues/1921.
const banner = {
js: `
import process from "node:process";
import {setImmediate, clearImmediate} from "node:timers";
import {Buffer} from "node:buffer";
import {createRequire as ___nfyCreateRequire} from "node:module";
import {fileURLToPath as ___nfyFileURLToPath} from "node:url";
import {dirname as ___nfyPathDirname} from "node:path";
Expand All @@ -94,21 +98,15 @@ interface GetNPMSpecifiersOptions {
basePath: string
functions: string[]
importMap: ParsedImportMap
referenceTypes: boolean
environment: 'production' | 'development'
rootPath: string
}

/**
* Parses a set of functions and returns a list of specifiers that correspond
* to npm modules.
*/
const getNPMSpecifiers = async ({
basePath,
functions,
importMap,
referenceTypes,
rootPath,
}: GetNPMSpecifiersOptions) => {
const getNPMSpecifiers = async ({ basePath, functions, importMap, environment, rootPath }: GetNPMSpecifiersOptions) => {
const baseURL = pathToFileURL(basePath)
const { reasons } = await nodeFileTrace(functions, {
base: rootPath,
Expand Down Expand Up @@ -191,7 +189,7 @@ const getNPMSpecifiers = async ({
if (isDirectDependency) {
npmSpecifiers.push({
specifier: packageName,
types: referenceTypes ? await safelyDetectTypes(path.join(basePath, filePath)) : undefined,
types: environment === 'development' ? await safelyDetectTypes(path.join(basePath, filePath)) : undefined,
})
}
}
Expand All @@ -208,7 +206,7 @@ interface VendorNPMSpecifiersOptions {
functions: string[]
importMap: ImportMap
logger: Logger
referenceTypes: boolean
environment: 'production' | 'development'
rootPath?: string
}

Expand All @@ -217,7 +215,7 @@ export const vendorNPMSpecifiers = async ({
directory,
functions,
importMap,
referenceTypes,
environment,
rootPath = basePath,
}: VendorNPMSpecifiersOptions) => {
// The directories that esbuild will use when resolving Node modules. We must
Expand All @@ -235,7 +233,7 @@ export const vendorNPMSpecifiers = async ({
basePath,
functions,
importMap: importMap.getContentsWithURLObjects(),
referenceTypes,
environment,
rootPath,
})

Expand All @@ -258,7 +256,6 @@ export const vendorNPMSpecifiers = async ({
}),
)
const entryPoints = ops.map(({ filePath }) => filePath)

// 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).
Expand All @@ -276,6 +273,12 @@ export const vendorNPMSpecifiers = async ({
splitting: true,
target: 'es2020',
write: false,
define:
environment === 'production'
? {
'process.env.NODE_ENV': '"production"',
}
: undefined,
})

await Promise.all(
Expand Down
2 changes: 1 addition & 1 deletion packages/edge-bundler/node/server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ const prepareServer = ({
functions: functions.map(({ path }) => path),
importMap,
logger,
referenceTypes: true,
environment: 'development',
rootPath,
})

Expand Down
10 changes: 10 additions & 0 deletions packages/edge-bundler/test/fixtures/tsx/functions/func1.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import React from 'react'

export default () => {
try {
// this is expected to fail
process.env.FOO
} catch {
return new Response(<p>Hello World</p>)
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 612776e

Please sign in to comment.