Bug: Parameters with typeof `steps` are not generated correctly

Xavientois commented 2 years ago

Is there an existing issue for this?

[X] I have searched the existing issues

Current behavior

Values passed to a parameter with the post_steps parameter type are generated incorrectly.

The builtin checkout command produces an empty object
A custom reusable command generates a name: property

This causes the generated config to fail when trying to run the pipeline

Minimum reproduction code

https://gist.github.com/Xavientois/21bb00fc854d75d3bab19905611d47a2

Steps to reproduce

No response

Expected behavior

Checkout would generate - checkout
The custom command would not include the name: property

CircleCI Config SDK version

0.10.1

Node.js version

16.18.0

In which operating systems have you tested?

[X] macOS
[ ] Windows
[ ] Linux

Other

No response

Jaryt commented 1 year ago

Hi! Thanks for opening this issue. It looks like this is an unexpected way of implementing post-steps which is causing the problem.

The WorkflowJob constructor has the parameter arguments, then two special arguments for pre steps and post steps.

constructor(
    job: Job | OrbRef<JobParameterLiteral>,
    parameters?: Exclude<WorkflowJobParameters, 'type'>,
    pre_steps?: StepsParameter,
    post_steps?: StepsParameter,
  )

So instead of defining the pre_steps in the parameters object yourself, you pass them along in the following arguments.

new CircleCI.Workflow('example', {}, [
    new CircleCI.workflow.WorkflowJob(vfcommon.jobs['install_and_build'], {}, [
      new CircleCI.commands.Checkout(),
      new CircleCI.reusable.ReusedCommand(vfcommon.commands['notify_slack'], {
        event: 'fail',
      }),
    ]),
  ]);

I also found an unexpected output where if you have undefined parameters, the pre and post steps will not be generated. The real issue here means that any "StepsParameter" might not be generating as expected. That's a good find. I'll change the title of this issue to reflect that. (Looks like your first title was correct 😄 )

Jaryt commented 1 year ago

I see now that there is a pre_steps are also on the parameters object. This is likely an error, but might be the preferable way. Happy to receive some input on which is better for the end user, as I can see pros and cons to both.