Skip to content

Commit

Permalink
fix: fix use headers/redirects that are set via the onPreDev of a plu…
Browse files Browse the repository at this point in the history
…gin (#6365)

* fix: fix use headers that are set via the onPreDev of a plugin

* chore: cleanup formatting

* chore: cleanup ts error

* chore: update tests and add predev redirects support

---------

Co-authored-by: Sean Roberts <[email protected]>
  • Loading branch information
sean-roberts and Sean Roberts authored Feb 1, 2024
1 parent 14f0d42 commit 35957ed
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 20 deletions.
10 changes: 7 additions & 3 deletions src/commands/dev/dev.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import process from 'process'

import { OptionValues, Option } from 'commander'
// @ts-expect-error TS(7016) FIXME: Could not find a declaration file for module '@net... Remove this comment to see the full error message
import { applyMutations } from '@netlify/config'

import { BLOBS_CONTEXT_VARIABLE, encodeBlobsContext, getBlobsContext } from '../../lib/blobs/blobs.js'
import { promptEditorHelper } from '../../lib/edge-functions/editor-helper.js'
Expand Down Expand Up @@ -153,7 +155,7 @@ export const dev = async (options: OptionValues, command: BaseCommand) => {

log(`${NETLIFYDEVWARN} Setting up local development server`)

const { configPath: configPathOverride } = await runDevTimeline({
const { configMutations, configPath: configPathOverride } = await runDevTimeline({
command,
options,
settings,
Expand All @@ -163,10 +165,12 @@ export const dev = async (options: OptionValues, command: BaseCommand) => {
},
})

const mutatedConfig = applyMutations(config, configMutations)

const functionsRegistry = await startFunctionsServer({
blobsContext,
command,
config,
config: mutatedConfig,
debug: options.debug,
settings,
site,
Expand Down Expand Up @@ -205,7 +209,7 @@ export const dev = async (options: OptionValues, command: BaseCommand) => {
await startProxyServer({
addonsUrls,
blobsContext,
config,
config: mutatedConfig,
configPath: configPathOverride,
debug: options.debug,
projectDir: command.workingDir,
Expand Down
3 changes: 2 additions & 1 deletion src/utils/headers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,12 @@ const getHeaderValues = function ({ values }) {
}

// @ts-expect-error TS(7031) FIXME: Binding element 'configPath' implicitly has an 'an... Remove this comment to see the full error message
export const parseHeaders = async function ({ configPath, headersFiles }): Promise<Header[]> {
export const parseHeaders = async function ({ config, configPath, headersFiles }): Promise<Header[]> {
const { errors, headers } = await parseAllHeaders({
headersFiles,
netlifyConfigPath: configPath,
minimal: false,
configHeaders: config?.headers || [],
})
handleHeadersErrors(errors)
return headers
Expand Down
28 changes: 24 additions & 4 deletions src/utils/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -423,8 +423,26 @@ const reqToURL = function (req, pathname) {

const MILLISEC_TO_SEC = 1e3

// @ts-expect-error TS(7031) FIXME: Binding element 'configPath' implicitly has an 'an... Remove this comment to see the full error message
const initializeProxy = async function ({ configPath, distDir, env, host, imageProxy, port, projectDir, siteInfo }) {
const initializeProxy = async function ({
// @ts-expect-error TS(7031) FIXME: Binding element 'configPath' implicitly has an 'any... Remove this comment to see the full error message
configPath,
// @ts-expect-error TS(7031) FIXME: Binding element 'distDir' implicitly has an 'any... Remove this comment to see the full error message
distDir,
// @ts-expect-error TS(7031) FIXME: Binding element 'env' implicitly has an 'any... Remove this comment to see the full error message
env,
// @ts-expect-error TS(7031) FIXME: Binding element 'host' implicitly has an 'any... Remove this comment to see the full error message
host,
// @ts-expect-error TS(7031) FIXME: Binding element 'imageProxy' implicitly has an 'any... Remove this comment to see the full error message
imageProxy,
// @ts-expect-error TS(7031) FIXME: Binding element 'port' implicitly has an 'any... Remove this comment to see the full error message
port,
// @ts-expect-error TS(7031) FIXME: Binding element 'projectDir' implicitly has an 'any... Remove this comment to see the full error message
projectDir,
// @ts-expect-error TS(7031) FIXME: Binding element 'siteInfo' implicitly has an 'any... Remove this comment to see the full error message
siteInfo,
// @ts-expect-error TS(7031) FIXME: Binding element 'config' implicitly has an 'any... Remove this comment to see the full error message
config,
}) {
const proxy = httpProxy.createProxyServer({
selfHandleResponse: true,
target: {
Expand All @@ -434,7 +452,7 @@ const initializeProxy = async function ({ configPath, distDir, env, host, imageP
})
const headersFiles = [...new Set([path.resolve(projectDir, '_headers'), path.resolve(distDir, '_headers')])]

let headers = await parseHeaders({ headersFiles, configPath })
let headers = await parseHeaders({ headersFiles, configPath, config })

const watchedHeadersFiles = configPath === undefined ? headersFiles : [...headersFiles, configPath]
onChanges(watchedHeadersFiles, async () => {
Expand All @@ -443,7 +461,7 @@ const initializeProxy = async function ({ configPath, distDir, env, host, imageP
`${NETLIFYDEVLOG} Reloading headers files from`,
existingHeadersFiles.map((headerFile) => path.relative(projectDir, headerFile)),
)
headers = await parseHeaders({ headersFiles, configPath })
headers = await parseHeaders({ headersFiles, configPath, config })
})

// @ts-expect-error TS(2339) FIXME: Property 'before' does not exist on type 'Server'.
Expand Down Expand Up @@ -853,9 +871,11 @@ export const startProxy = async function ({
configPath,
siteInfo,
imageProxy,
config,
})

const rewriter = await createRewriter({
config,
distDir: settings.dist,
projectDir,
jwtSecret: settings.jwtSecret,
Expand Down
4 changes: 2 additions & 2 deletions src/utils/redirects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ import { NETLIFYDEVERR, log } from './command-helpers.js'
// Parse, normalize and validate all redirects from `_redirects` files
// and `netlify.toml`
// @ts-expect-error TS(7031) FIXME: Binding element 'configPath' implicitly has an 'an... Remove this comment to see the full error message
export const parseRedirects = async function ({ configPath, redirectsFiles }) {
// @ts-expect-error TS(2345) FIXME: Argument of type '{ redirectsFiles: any; netlifyCo... Remove this comment to see the full error message
export const parseRedirects = async function ({ config, configPath, redirectsFiles }) {
const { errors, redirects } = await parseAllRedirects({
redirectsFiles,
netlifyConfigPath: configPath,
minimal: false,
configRedirects: config?.redirects || [],
})
handleRedirectParsingErrors(errors)
// @ts-expect-error TS(2345) FIXME: Argument of type '({ conditions: { country, langua... Remove this comment to see the full error message
Expand Down
6 changes: 4 additions & 2 deletions src/utils/rules-proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ export const getLanguage = function (headers) {
}

export const createRewriter = async function ({
// @ts-expect-error TS(7031) FIXME: Binding element 'config' implicitly has an 'an... Remove this comment to see the full error message
config,
// @ts-expect-error TS(7031) FIXME: Binding element 'configPath' implicitly has an 'an... Remove this comment to see the full error message
configPath,
// @ts-expect-error TS(7031) FIXME: Binding element 'distDir' implicitly has an 'any' ... Remove this comment to see the full error message
Expand All @@ -56,7 +58,7 @@ export const createRewriter = async function ({
// @ts-expect-error TS(7034) FIXME: Variable 'matcher' implicitly has type 'any' in so... Remove this comment to see the full error message
let matcher = null
const redirectsFiles = [...new Set([path.resolve(distDir, '_redirects'), path.resolve(projectDir, '_redirects')])]
let redirects = await parseRedirects({ redirectsFiles, configPath })
let redirects = await parseRedirects({ config, redirectsFiles, configPath })

const watchedRedirectFiles = configPath === undefined ? redirectsFiles : [...redirectsFiles, configPath]
onChanges(watchedRedirectFiles, async () => {
Expand All @@ -65,7 +67,7 @@ export const createRewriter = async function ({
`${NETLIFYDEVLOG} Reloading redirect rules from`,
existingRedirectsFiles.map((redirectFile) => path.relative(projectDir, redirectFile)),
)
redirects = await parseRedirects({ redirectsFiles, configPath })
redirects = await parseRedirects({ config, redirectsFiles, configPath })
matcher = null
})

Expand Down
4 changes: 2 additions & 2 deletions src/utils/run-build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,13 @@ export const runNetlifyBuild = async ({ command, env = {}, options, settings, ti
}

// Run Netlify Build using the `startDev` entry point.
const { error: startDevError, success } = await startDev(devCommand, startDevOptions)
const { configMutations, error: startDevError, success } = await startDev(devCommand, startDevOptions)

if (!success && startDevError) {
error(`Could not start local development server\n\n${startDevError.message}\n\n${startDevError.stack}`)
}

return {}
return { configMutations }
}

/**
Expand Down
28 changes: 22 additions & 6 deletions tests/integration/commands/dev/dev-forms-and-redirects.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,20 @@ describe.concurrent('commands/dev-forms-and-redirects', () => {
throw new Error("I should not run");
},
onPreDev: () => {
onPreDev: ({netlifyConfig}) => {
netlifyConfig.headers.push({
for: '/*',
values: {
'x-test': 'value'
}
});
netlifyConfig.redirects.push({
from: '/baz/*',
to: 'http://localhost:${userServerPort}/:splat',
status: 200,
});
const server = http.createServer((_, res) => res.end("Hello world"));
server.listen(${userServerPort}, "localhost", () => {
Expand Down Expand Up @@ -418,12 +431,15 @@ describe.concurrent('commands/dev-forms-and-redirects', () => {
await builder.buildAsync()

await withDevServer({ cwd: builder.directory }, async (server) => {
const [response1, response2] = await Promise.all([
fetch(`${server.url}/foo`).then((res) => res.text()),
fetch(`http://localhost:${userServerPort}`).then((res) => res.text()),
const [response1, response2, response3] = await Promise.all([
fetch(`${server.url}/foo`),
fetch(`http://localhost:${userServerPort}`),
fetch(`${server.url}/baz/path`),
])
t.expect(response1).toEqual('<html><h1>foo')
t.expect(response2).toEqual('Hello world')
t.expect(await response1.text()).toEqual('<html><h1>foo')
t.expect(response1.headers.get('x-test')).toEqual('value')
t.expect(await response2.text()).toEqual('Hello world')
t.expect(await response3.text()).toEqual('Hello world')
})
})
})
Expand Down
1 change: 1 addition & 0 deletions tests/integration/rules-proxy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ describe('rules-proxy', () => {
await builder.build()

const rewriter = await createRewriter({
config: {},
distDir: builder.directory,
projectDir: builder.directory,
jwtSecret: '',
Expand Down

2 comments on commit 35957ed

@github-actions
Copy link

Choose a reason for hiding this comment

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

📊 Benchmark results

  • Dependency count: 1,282
  • Package size: 280 MB
  • Number of ts-expect-error directives: 1,181

@github-actions
Copy link

Choose a reason for hiding this comment

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

📊 Benchmark results

  • Dependency count: 1,282
  • Package size: 280 MB
  • Number of ts-expect-error directives: 1,181

Please sign in to comment.