Skip to content

Commit

Permalink
fix: reintroduce watch and direct import of config files (#2634)
Browse files Browse the repository at this point in the history
`engine-cli` used to have 3 config modes:
1. require - was actually doing a direct `import()` to the config (was a `require()` call before the move to esm)
2. watch - bundles config files into cjs bundles. this was (and still is) broken when the config module, or any of the files it imports use esm-specific syntax (e.g import.meta).
3. fresh - this used `import()` via a freshly created worker_thread. it allowed getting the "current" value of a config file (in dev time), meaning you don't have to watch. it got broken when we moved to esm.

#2629 removed the first two and fixed the third. this caused a speed regression downstream, as tests imported configs over and over in a non-cached manner.

with this PR, we now have 3 modes again:
1. "import" - same functionality as before, but a non-confusing name
2. "watch" - same functionality as before. still broken when using native esm syntax
3. fresh - got fixed so it actually works again. not used unless cli flag is passed

"import" is now the default unless cli is in watch mode (where "watch" gets be the default)
AviVahl authored Jan 20, 2025

Verified

This commit was signed with the committer’s verified signature.
987Nabil Nabil Abdel-Hafeez
1 parent 3b6f97f commit 8521f8d
Showing 5 changed files with 9 additions and 9 deletions.
6 changes: 3 additions & 3 deletions packages/engine-cli/src/cli.ts
Original file line number Diff line number Diff line change
@@ -110,13 +110,13 @@ async function engine() {
},
configLoadingMode: {
type: (value: string) => {
if (value === 'fresh' || value === 'watch' || value === 'require') {
if (value === 'fresh' || value === 'watch' || value === 'import') {
return value;
} else {
throw new Error(`Invalid config loading mode: ${value}`);
}
},
description: 'Config loading mode (fresh, watch, require)',
description: 'Config loading mode (fresh, watch, import)',
default: undefined,
},
verbose: {
@@ -195,7 +195,7 @@ async function engine() {
} else {
const dev = argv.flags.dev ?? argv.flags.watch;
const run = argv.flags.run ?? dev;
const configLoadingMode = argv.flags.configLoadingMode ?? (argv.flags.watch ? 'watch' : 'require');
const configLoadingMode = argv.flags.configLoadingMode ?? (argv.flags.watch ? 'watch' : 'import');
const runtimeArgs = argv.flags.runtimeArgs;
addRuntimeArgsFlagsFromEngineConfig(engineConfig, argv.flags, runtimeArgs);

2 changes: 1 addition & 1 deletion packages/engine-cli/src/engine-build.ts
Original file line number Diff line number Diff line change
@@ -64,7 +64,7 @@ export async function runEngine({
runtimeArgs = {},
writeMetadataFiles = !watch,
publicConfigsRoute = 'configs',
configLoadingMode = 'require',
configLoadingMode = 'import',
staticBuild = false,
title,
}: RunEngineOptions = {}): Promise<{
2 changes: 1 addition & 1 deletion packages/engine-cli/src/import-fresh.ts
Original file line number Diff line number Diff line change
@@ -14,7 +14,7 @@ import { Worker, isMainThread, parentPort, workerData } from 'node:worker_thread
export async function importFresh(filePath: string[], exportSymbolName?: string): Promise<unknown[]>;
export async function importFresh(filePath: string, exportSymbolName?: string): Promise<unknown>;
export async function importFresh(filePath: string | string[], exportSymbolName = 'default'): Promise<unknown> {
const worker = new Worker(import.meta.url, {
const worker = new Worker(new URL(import.meta.url), {
workerData: { filePath, exportSymbolName } satisfies ImportFreshWorkerData,
// doesn't seem to inherit two levels deep (Worker from Worker)
execArgv: [...process.execArgv],
4 changes: 2 additions & 2 deletions packages/engine-cli/src/launch-dashboard-server.ts
Original file line number Diff line number Diff line change
@@ -7,7 +7,7 @@ import { runLocalNodeManager } from './run-local-mode-manager.js';
import { NodeConfigManager } from './node-config-manager.js';
import type { StaticConfig } from './types.js';

export type ConfigLoadingMode = 'fresh' | 'watch' | 'require';
export type ConfigLoadingMode = 'fresh' | 'watch' | 'import';

export async function launchDashboardServer(
rootDir: string,
@@ -104,7 +104,7 @@ function runOnDemandSingleEnvironment(
featureEnvironmentsMapping: FeatureEnvironmentMapping,
configMapping: ConfigurationEnvironmentMapping,
outputPath: string,
configLoadingMode: 'fresh' | 'watch' | 'require',
configLoadingMode: 'fresh' | 'watch' | 'import',
waitForBuildReady?: (cb: () => void) => boolean,
buildConditions?: string[],
extensions?: string[],
4 changes: 2 additions & 2 deletions packages/runtime-node/src/node-env-manager.ts
Original file line number Diff line number Diff line change
@@ -48,7 +48,7 @@ export class NodeEnvManager implements IDisposable {
private importMeta: { url: string },
private featureEnvironmentsMapping: FeatureEnvironmentMapping,
private configMapping: ConfigurationEnvironmentMapping,
private loadModules: (modulePaths: string[]) => Promise<unknown> = requireModules,
private loadModules: (modulePaths: string[]) => Promise<unknown> = importModules,
) {}
public async autoLaunch(runtimeOptions = parseRuntimeOptions(), serverOptions: ILaunchHttpServerOptions = {}) {
process.env.ENGINE_FLOW_V2_DIST_URL = this.importMeta.url;
@@ -207,7 +207,7 @@ export class NodeEnvManager implements IDisposable {
}
}

async function requireModules(modulePaths: string[]) {
async function importModules(modulePaths: string[]) {
const loadedModules: unknown[] = [];
for (const modulePath of modulePaths) {
const importedModule = await import(pathToFileURL(modulePath).href);

0 comments on commit 8521f8d

Please sign in to comment.