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

fix(builder): Fix output clash on var steps #1218

Merged
merged 14 commits into from
Aug 6, 2024
Merged
16 changes: 16 additions & 0 deletions packages/builder/src/definition.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,22 @@ describe('ChainDefinition', () => {
});
});

it('does not have output clash on var or settings', () => {
const rawDef: RawChainDefinition = {
name: 'test',
version: '1.0.0',
var: {
a: { b: '<%= settings.c %>', description: 'desc 1' },
d: { c: '1234', description: 'desc 2' },
},
deploy: {
woot: { artifact: 'wohoo', args: ['<%= settings.b %>'], depends: [] },
},
};

expect(new ChainDefinition(rawDef)).toBeTruthy;
});

describe('validatePackageName()', () => {
it('verifies the name is not too short', () => {
expect(() => validatePackageName('hh')).toThrowError('must be at least');
Expand Down
2 changes: 1 addition & 1 deletion packages/builder/src/definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export class ChainDefinition {

for (const output of actionOutputs) {
debug(`deps: ${fullActionName} provides ${output}`);
if (!this.dependencyFor.has(output)) {
if (!this.dependencyFor.has(output) || action === 'setting' || action === 'var') {
Copy link
Member

Choose a reason for hiding this comment

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

This wouldn't work weird when having a var that uses another var? Meaning that it wont be able to find correctly the dependency?

this.dependencyFor.set(output, fullActionName);
} else {
throw new Error(`output clash: both ${this.dependencyFor.get(output)} and ${fullActionName} output ${output}`);
Expand Down
Loading