Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions .changeset/fix-diamond-dependency-double-import.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"varlock": patch
"@varlock/nextjs-integration": patch
---

Fix diamond dependency handling when the same schema is imported via multiple paths. Previously, duplicate imports caused plugin init decorators to run twice ("Instance already initialized" error). Now, duplicate imports create lightweight `ImportAliasSource` nodes that appear at the correct precedence position without re-initializing the source. This correctly handles different importKeys subsets across import sites and preserves override semantics matching non-deduplicated behavior. Also adds `type` field to serialized source entries for easier filtering.
6 changes: 5 additions & 1 deletion .github/workflows/smoke-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,12 @@ jobs:
is-release-pr: ${{ steps.check.outputs.is-release-pr }}
steps:
- id: check
env:
PR_TITLE: ${{ github.event.pull_request.title }}
HAS_LABEL: ${{ contains(github.event.pull_request.labels.*.name, 'smoke-tests') }}
EVENT_NAME: ${{ github.event_name }}
run: |
if [[ "${{ github.event.pull_request.title }}" == *"[Changesets]"* ]] || [[ "${{ contains(github.event.pull_request.labels.*.name, 'smoke-tests') }}" == "true" ]] || [[ "${{ github.event_name }}" == "workflow_dispatch" ]] || [[ "${{ github.event_name }}" == "push" ]]; then
if [[ "$PR_TITLE" == *"[Changesets]"* ]] || [[ "$HAS_LABEL" == "true" ]] || [[ "$EVENT_NAME" == "workflow_dispatch" ]] || [[ "$EVENT_NAME" == "push" ]]; then
echo "is-release-pr=true" >> $GITHUB_OUTPUT
else
echo "is-release-pr=false" >> $GITHUB_OUTPUT
Expand Down
8 changes: 4 additions & 4 deletions bun.lock

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

4 changes: 1 addition & 3 deletions packages/integrations/nextjs/src/next-env-compat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ let rootDir: string | undefined;
// a list of filenames loaded, for example: `Environments: .env, .env.development`
function getVarlockSourcesAsLoadedEnvFiles(): LoadedEnvFiles {
const envFilesLabels = varlockLoadedEnv.sources
// TODO expose more info so we can filter out disabled sources
// and maybe show relative paths
.filter((s) => s.enabled && !s.label.startsWith('directory -'))
.filter((s) => s.enabled && s.type !== 'container' && s.type !== 'import-alias')
.map((s) => s.label);
if (envFilesLabels.length) {
// this adds an additional line, below the list of files
Expand Down
5 changes: 4 additions & 1 deletion packages/tsconfig/plugin.tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
{
"extends": "./base.tsconfig.json"
"extends": "./base.tsconfig.json",
"compilerOptions": {
"types": ["node"]
}
}
9 changes: 4 additions & 5 deletions packages/varlock/src/env-graph/lib/config-item.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,20 +45,19 @@ export class ConfigItem {
}

/**
* fetch ordered list of definitions for this item, by following up sorted data sources list
* fetch ordered list of definitions for this item, by following the sorted definition sources list
* internal defs (builtins) are appended last (lowest priority)
*/
get defs() {
// TODO: this is somewhat inneficient, because some of the checks on the data source follow up the parent chain
// we may want to cache the definition list at some point when loading is complete
// although we need it to be dynamic during the loading process when doing any early resolution of the envFlag
const defs: Array<ConfigItemDefAndSource> = [];
for (const source of this.envGraph.sortedDataSources) {
for (const { source, importKeys } of this.envGraph.sortedDefinitionSources) {
if (!source.configItemDefs[this.key]) continue;
if (source.disabled) continue;
if (source.importKeys && !source.importKeys.includes(this.key)) continue;
const itemDef = source.configItemDefs[this.key];
if (itemDef) defs.push({ itemDef, source });
if (importKeys && !importKeys.includes(this.key)) continue;
defs.push({ itemDef: source.configItemDefs[this.key], source });
}
defs.push(...this._internalDefs);
return defs;
Expand Down
57 changes: 52 additions & 5 deletions packages/varlock/src/env-graph/lib/data-source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ const DATA_SOURCE_TYPES = Object.freeze({
},
container: {
},
'import-alias': {
},
});
type DataSourceType = keyof typeof DATA_SOURCE_TYPES;

Expand Down Expand Up @@ -384,6 +386,18 @@ export abstract class EnvGraphDataSource {
const fileName = path.basename(fullImportPath);

// TODO: might be nice to move this logic somewhere else
// Check for diamond dependency: if this path was already loaded,
// add an alias instead of re-loading (prevents double plugin init)
const existingSource = this.graph.getLoadedImportSource(fullImportPath);
if (existingSource) {
// eslint-disable-next-line no-use-before-define
await this.addChild(new ImportAliasSource(existingSource), {
isImport: true, importKeys, isConditionallyEnabled,
});
this.graph.registerItemsForImport(existingSource, this, importKeys);
continue;
}

if (this.graph.virtualImports) {
if (importPath.endsWith('/')) {
const dirExists = Object.keys(this.graph.virtualImports).some((p) => p.startsWith(fullImportPath));
Expand All @@ -393,9 +407,11 @@ export abstract class EnvGraphDataSource {
return;
}
// eslint-disable-next-line no-use-before-define
await this.addChild(new DirectoryDataSource(fullImportPath), {
const dirChild = new DirectoryDataSource(fullImportPath);
await this.addChild(dirChild, {
isImport: true, importKeys, isConditionallyEnabled,
});
this.graph.recordLoadedImportPath(fullImportPath, dirChild);
} else {
const fileExists = this.graph.virtualImports[fullImportPath];
if (!fileExists && allowMissing) continue;
Expand All @@ -404,10 +420,11 @@ export abstract class EnvGraphDataSource {
return;
}
// eslint-disable-next-line no-use-before-define
const source = new DotEnvFileDataSource(fullImportPath, {
const fileChild = new DotEnvFileDataSource(fullImportPath, {
overrideContents: this.graph.virtualImports[fullImportPath],
});
await this.addChild(source, { isImport: true, importKeys, isConditionallyEnabled });
await this.addChild(fileChild, { isImport: true, importKeys, isConditionallyEnabled });
this.graph.recordLoadedImportPath(fullImportPath, fileChild);
}
} else {
const fsStat = await tryCatch(async () => fs.stat(fullImportPath), (_err) => {
Expand All @@ -424,9 +441,11 @@ export abstract class EnvGraphDataSource {
if (importPath.endsWith('/')) {
if (fsStat.isDirectory()) {
// eslint-disable-next-line no-use-before-define
await this.addChild(new DirectoryDataSource(fullImportPath), {
const dirChild = new DirectoryDataSource(fullImportPath);
await this.addChild(dirChild, {
isImport: true, importKeys, isConditionallyEnabled,
});
this.graph.recordLoadedImportPath(fullImportPath, dirChild);
} else {
this._loadingError = new Error(`Imported path ending with "/" is not a directory: ${fullImportPath}`);
return;
Expand All @@ -442,9 +461,11 @@ export abstract class EnvGraphDataSource {
}
// TODO: once we have more file types, here we would detect the type and import it correctly
// eslint-disable-next-line no-use-before-define
await this.addChild(new DotEnvFileDataSource(fullImportPath), {
const fileChild = new DotEnvFileDataSource(fullImportPath);
await this.addChild(fileChild, {
isImport: true, importKeys, isConditionallyEnabled,
});
this.graph.recordLoadedImportPath(fullImportPath, fileChild);
}
}
} else if (importPath.startsWith('http://') || importPath.startsWith('https://')) {
Expand Down Expand Up @@ -544,6 +565,32 @@ export abstract class EnvGraphDataSource {
}
}

/**
* Lightweight alias created when the same path is imported from multiple locations.
* Lives in the tree like any other child (has its own importMeta/parent) but delegates
* definitions to the original source. Has no rootDecorators or configItemDefs of its own,
* so sortedDataSources consumers (plugin init, error collection, etc.) naturally skip it.
*/
export class ImportAliasSource extends EnvGraphDataSource {
type = 'import-alias' as const;
typeLabel = 'import-alias';

constructor(
/** the real data source this alias points to */
readonly original: EnvGraphDataSource,
) {
super();
}

get label() { return `re-import of ${this.original.label}`; }

// no-op — the original was already fully initialized
// eslint-disable-next-line @typescript-eslint/no-empty-function
async _finishInit() {}
// eslint-disable-next-line @typescript-eslint/no-empty-function
async _processImports() {}
}

export abstract class FileBasedDataSource extends EnvGraphDataSource {
fullPath: string;
fileName: string;
Expand Down
99 changes: 98 additions & 1 deletion packages/varlock/src/env-graph/lib/env-graph.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import _ from '@env-spec/utils/my-dash';
import path from 'node:path';
import { ConfigItem } from './config-item';
import { EnvGraphDataSource, FileBasedDataSource } from './data-source';
import { EnvGraphDataSource, FileBasedDataSource, ImportAliasSource } from './data-source';

import { BaseResolvers, createResolver, type ResolverChildClass } from './resolver';
import { BaseDataTypes, type EnvGraphDataTypeFactory } from './data-types';
Expand All @@ -27,9 +27,18 @@ export type SerializedEnvGraphErrors = {
root?: Array<string>;
};

/** Entry in the sorted definition sources list — pairs a data source with the importKeys
* filter that applies at that specific position in the precedence chain */
export type DefinitionSourceEntry = {
source: EnvGraphDataSource;
/** importKeys filter for this position (undefined = all keys visible) */
importKeys?: Array<string>;
};

export type SerializedEnvGraph = {
basePath?: string;
sources: Array<{
type: string;
label: string;
enabled: boolean;
path?: string;
Expand Down Expand Up @@ -68,6 +77,63 @@ export class EnvGraph {
configSchema: Record<string, ConfigItem> = {};


/**
* Tracks directory/file paths that have already been loaded as imports.
* Maps each import path to the data source that was created for it.
* Used to prevent diamond-dependency re-imports (same schema imported via multiple paths),
* which would otherwise cause plugin init decorators to run multiple times.
*/
private _loadedImportPaths = new Map<string, EnvGraphDataSource>();

/** Returns the existing source for a path if already loaded, or undefined */
getLoadedImportSource(importPath: string): EnvGraphDataSource | undefined {
return this._loadedImportPaths.get(importPath);
}

/** Records the data source that was created for an import path */
recordLoadedImportPath(importPath: string, dataSource: EnvGraphDataSource) {
this._loadedImportPaths.set(importPath, dataSource);
}

/**
* Register ConfigItems for keys visible through an import
* that may not have been registered during the original source's finishInit.
*/
registerItemsForImport(
source: EnvGraphDataSource,
importSite: EnvGraphDataSource,
importKeys?: Array<string>,
) {
// Compute effective importKeys: intersection of import filter and importSite's parent chain
const siteKeys = importSite.importKeys;
let effectiveKeys: Array<string> | undefined;
const hasFilter = importKeys && importKeys.length > 0;
if (hasFilter && siteKeys?.length) {
effectiveKeys = importKeys.filter((k) => siteKeys.includes(k));
} else if (hasFilter) {
effectiveKeys = importKeys;
} else {
effectiveKeys = siteKeys;
}

for (const s of this._getDescendants(source)) {
const keys = effectiveKeys || _.keys(s.configItemDefs);
for (const itemKey of keys) {
if (!s.configItemDefs[itemKey]) continue;
this.configSchema[itemKey] ??= new ConfigItem(this, itemKey);
}
}
}

/** Get a data source and all its descendants (DFS) */
private _getDescendants(source: EnvGraphDataSource): Array<EnvGraphDataSource> {
const result: Array<EnvGraphDataSource> = [source];
for (const child of source.children) {
result.push(...this._getDescendants(child));
}
return result;
}

/** virtual imports for testing */
virtualImports?: Record<string, string>;
setVirtualImports(basePath: string, files: Record<string, string>) {
Expand All @@ -85,6 +151,36 @@ export class EnvGraph {
return this.rootDataSource ? getSourceAndChildren(this.rootDataSource) : [];
}

/**
* Precedence-ordered list of definition sources, used by ConfigItem.defs.
*
* Unlike `sortedDataSources` (which contains each real source exactly once),
* this list can contain the same source multiple times at different positions
* when it's imported from multiple locations (diamond dependency). Each entry
* carries its own `importKeys` filter for that specific import context.
*
* Built from `sortedDataSources` by expanding `ImportAliasSource` nodes into
* the original source's full subtree at the alias's precedence position.
*/
get sortedDefinitionSources(): Array<DefinitionSourceEntry> {
const result: Array<DefinitionSourceEntry> = [];

for (const source of this.sortedDataSources) {
if (source instanceof ImportAliasSource) {
// Alias: expand to the original source's subtree at this position,
// using the alias's importKeys (derived from its own parent chain)
const importKeys = source.importKeys;
for (const descendant of this._getDescendants(source.original)) {
result.push({ source: descendant, importKeys });
}
} else {
result.push({ source, importKeys: source.importKeys });
}
}

return result;
}

registeredResolverFunctions: Record<string, ResolverChildClass> = {};
registerResolver(resolverClass: ResolverChildClass) {
// because its a class, we can't use `name`
Expand Down Expand Up @@ -439,6 +535,7 @@ export class EnvGraph {
};
for (const source of this.sortedDataSources) {
serializedGraph.sources.push({
type: source.type,
label: source.label,
enabled: !source.disabled,
path: source instanceof FileBasedDataSource ? path.relative(this.basePath ?? '', source.fullPath) : undefined,
Expand Down
Loading
Loading