Inaccurate TypeScript types

[jednano]

There are a number of inaccuracies in the TypeScript types that are throwing errors, even when the configuration is accurate. For example, If I wanted to add an append action like this:

{
  type: 'append',
  path: './foo.ts',
  separator: '',
  template: "export * from './{{name}';\n"
}

This is an accurate AddActionConfig, but the AddActionConfig interface declares that the templateFile is a required property. Actually, either the template or the templateFile is required, but not both at the same time. The way to properly type this is to create separate interfaces for each one of these scenarios, something like this:

interface AddActionConfigBase extends ActionConfig {
  type: 'add';
  path: string;
  skipIfExists?: boolean;
}

interface AddActionConfigWithTemplate extends AddActionConfigBase {
  template: string;
}

interface AddActionConfigWithTemplateFile extends AddActionConfigBase {
  templateFile: string;
}

interface AddActionConfigWithTransform extends AddActionConfigBase {
  transform: TransformFn<AddActionConfig>;
  template?: string;
  templateFile?: string;
}

export type AddActionConfig =
  | AddActionConfigWithTemplate
  | AddActionConfigWithTemplateFile
  | AddActionConfigWithTransform;

Now, I'm not 100% sure these are the actual requirements for these configurations, as the documentation doesn't really indicate the optionality of these properties, but I think you can see from my example how to properly define these scenarios.

The same issue presents itself in the ModifyActionConfig and potentially others.

[armand1m]

Hey! Thanks for bringing this up. I believe both should be supported as you stated, is there any context we might be missing here? @crutchcorn

[davidmwhynot]

I opened a PR with a fix for this (#196). @armand1m What needs to happen to get that merged in?

[crutchcorn]

Apologies for the delays, y'all! December was an extremely rough month on me and I took a bit of a break from some of my projects.

Just took a look at the code changes in #196 and it all looks like fantastic changes - thanks so much @davidmwhynot !

I'll merge and publish in just a few hours (at most)

[jednano]

I just want to reiterate that “The same issue presents itself in the ModifyActionConfig and potentially others.”

The PR only fixes the “add” action.

[crutchcorn]

@jedmao would you be willing to open a PR for the other actions as well?

[jednano]

I can’t, sorry.

[derekdon]

Does this need to return promise of a string?

export type CustomActionFunction = (
	answers: object,
	config?: ActionConfig,
	plopfileApi?: NodePlopAPI
) => Promise<string> | string; // Check return type?

[davidmwhynot]

I'm not super familiar with what is supported for the other actions besides the "add" action as my use-case for plop is pretty simple and only requires the use of "add" actions.
If this changes in the future, I'm sure I'll run into some type errors along the way and will open another PR to correct the issues. Until that happens though, I unfortunately do not have time to fix the other problems mentioned in this issue.

[amwmedia]

@derekdon the return type you list looks correct to me. A CustomActionFunction can return a status string or a promise that resolves to a status string in the case where your CustomActionFunction needs to do async tasks.

[crutchcorn]

This issue is being fixed (for all types, not just add), and with type tests to verify, in node-plop@0.13 and plop@3. Tracking PR: #212

[crutchcorn]

https://github.com/plopjs/plop/releases/tag/v3.0.0

These issues (for all action types) should now be solved as-of node-plop@0.13.0 and plop@3. Please open a new issue if new/other typings issues arise