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

Add support for other enquirer prompts. #9038

Open
1 task
jskrzypek opened this issue Feb 19, 2022 · 23 comments
Open
1 task

Add support for other enquirer prompts. #9038

jskrzypek opened this issue Feb 19, 2022 · 23 comments
Assignees
Labels
scope: core core nx functionality type: feature

Comments

@jskrzypek
Copy link

jskrzypek commented Feb 19, 2022

Description

Enquirer is used by @nrwl/tao to handle the prompts for generator options via the schema. Currently only the input, select, multiselect, confirm, & numeral prompts are supported.

However, there are several other prompt types that would be very convenient to have for some generators, such as

  • list prompts for when you want an array of items, but don't want to constrain the options, like multiselect does
  • autocomplete prompts for when you want to suggest options, but not constrain input to them, like select does
  • invisible or password might be good for secrets, if you have a generator that needs to access cloud resources/ private repos.
  • less obvious how it would be implmented, but snippet prompts would provide a very nice & clean way to make templating for generators that use EJS templates interactive

Motivation

I initially thought of this because I wanted to create a generator that would be able to add targets for running @nx-tools/nx-docker to my apps. I was starting to lay it out and I realized that for a tags input, there's no effective way to prompt for a free-form list, either I make the schema option demand a single delimited string, and then manually split it in the generator's code, or I don't provide a prompt. The fact that enquirer provides a list prompt that would be about a 5-line implementation was too tantalizing to ignore.

After looking at enquirer, I realized that several other prompt types might lend themselves very nicely to nx's patterns (described above)

Suggested Implementation

The list prompt type should be very straightforward to implement, as would the autocomplete, invisible, or password prompts; just pass through the prompt type (if it's one of those) and add some checks to validate that the type of the property matches what the prompt type returns.

Alternate Implementations

The snippet prompt for doing interactive templating would require significantly more effort, and may not be feasible, but I imagining that it could probably be implemented look at the options allowed by the schema and then create a substitutions object to be used by EJS to render the template file into a template string that you can be passed to the snippet prompt. The problem here lies in the fact that the snippet prompt only returns the single, rendered template string and doesn't have support for multiple templates or surfacing only what the filled in values were.

To have this jive well with the nx cli approach, you'd need to write a custom prompt based on the approach of the snippet prompt. It would need to be made aware of all of template files, maybe showing them all at once at once (ugly), iterate over them, or add some additional interactions to toggle between templates that still need to be filled in. You'd also want to have it return just the object of options as filled in by the user, so as not to disrupt the current flow of the generators (i.e. collect/resolve the prompted options & only then run generator logic – e.g. calling @nrwl/devkit's generateFiles()).

@FrozenPandaz FrozenPandaz added the scope: core core nx functionality label Mar 14, 2022
@FrozenPandaz
Copy link
Collaborator

We can add more support for different enquirer types. Would you be open to contributing them?

@FrozenPandaz FrozenPandaz added the community This is a good first issue for contributing label Mar 14, 2022
@nzacca
Copy link

nzacca commented May 22, 2022

If it's ok with @jskrzypek , I'd like to offer to take this on.

@FrozenPandaz I do have some questions about what we want to accomplish here:

  • Are we looking to have something generic where folks can use any enquirer prompt or did we just want to hardcode support for the ones that @jskrzypek listed above?
  • Are there any considerations that I should keep in mind when adding support for other enquirer types? For example, does this affect NX Console in any way? I've never used NX Console so this might be a silly question.
  • I didn't see any tests for the existing prompts, is this something that we need? Is this something that can be tested?

@jskrzypek
Copy link
Author

jskrzypek commented May 22, 2022 via email

@nzacca
Copy link

nzacca commented May 22, 2022

@jskrzypek Excellent. I'll get started on this once we hear back from the Nx team.

In the meantime, I have prototyped the following as a possible solution which seems to work well:

// schema.json
{
  "properties": {
    "foo": {
      "type": "object",
      "x-prompt": {
        "message": "Question?",
        "type": "custom",
        "definition": "dist/out-tsc/tools/generators/foo/foo.js#myCustomThing"
      }
    }
  }
}

// foo.js
export const myCustomThing = () => {
  // Return prompt definition
  return {
    type: "snippet",
    fields: [
      {
        name: "bar",
        message: "bar"
      }
    ],
    template: `
{
  "name": "\${bar}"
}`,
  }
}

The custom function can also be async if necessary. This essentially acts as an "escape hatch" that allows you to drop out of nx and into enquirer. That said...

The downside of this is, if in future Nx wants to swap out enquirer for something else (ex. this has already happened with enquirer over inquirer previously) then this becomes a much more difficult migration for folks. Because of this, I'm inclined to lean more toward wrapping enquirer prompts in interfaces that Nx can control as is currently done. The answer to this really depends on how much the Nx team wants to lean into the enquirer api.

Just as a thought, maybe as an "unofficial" workaround, folks can attach additional enquirer properties to prompt properties that will be added but will have no guarantee from the Nx team to work in future. Essentially a "use at your own risk". I'm not saying this is good or bad one way or the other but just offering thoughts for discussion.

@nzacca
Copy link

nzacca commented May 29, 2022

@FrozenPandaz

Friendly ping for feedback from the Nx team. Just want to know if the approach above sounds OK to you before I proceed.

@nzacca
Copy link

nzacca commented Jun 20, 2022

@FrozenPandaz

Friendly follow up. Just looking for feedback/direction. Cheers!

@nzacca
Copy link

nzacca commented Jul 16, 2022

@FrozenPandaz

Hello again! Following up to see if the above sounds ok. If so, I will proceed with the implementation and PR.

nzacca added a commit to nzacca/nx that referenced this issue Aug 31, 2022
* Allows defining built-in prompts supported by enquirer

ISSUES CLOSED: nrwl#9038
nzacca added a commit to nzacca/nx that referenced this issue Aug 31, 2022
* Allows defining built-in prompts supported by enquirer
ISSUES CLOSED: nrwl#9038
@nzacca
Copy link

nzacca commented Aug 31, 2022

I have gone ahead and submitted a PR for this. I hope that is OK. I really wanted to get some feedback from the Nx team on this before starting it but I think what was done here is a reasonable place to start. My team is in need of this so hopefully we can get this moved forward.

Cheers!

@FrozenPandaz
Copy link
Collaborator

@nzacca It was okay to open the PR. Thank you for attempting the fix.

I think CustomPrompts will be much more involved. It needs to handle published Plugins as well as Local Plugins.

It's likely best for @AgentEnder or someone from the core team to take on that implementation.

Do you have an idea for list, password/invisible?

We've already added autocomplete so hopefully that satisfies that prompt type.

@nzacca
Copy link

nzacca commented Sep 22, 2022

@FrozenPandaz

Thank you very much for the response. I proceeded under the assumption that it would be OK to open this up for all enquirer prompts. The solution I proposed above works for local but I didn't think about published packages. I haven't had a chance to think about how that would work but I don't think it would be far off from what I implemented. For example:

// Local prompt example - this is what I implemented
{
  "$schema": "http://json-schema.org/schema",
  "id": "my-generator",
  "type": "object",
  "properties": {
    "age": {
      "type": "boolean",
      "description": "Snippet Example",
      "x-prompt": {
        "type": "custom",
        "source": "prompts/foo"
      }
    }
  }
}

// Proposed node_modules example
{
  "$schema": "http://json-schema.org/schema",
  "id": "my-generator",
  "type": "object",
  "properties": {
    "age": {
      "type": "boolean",
      "description": "Snippet Example",
      "x-prompt": {
        "type": "custom",
        "source": "@somepackage",
        "prompt": "fooPrompt"
      }
    }
  }
}

Could you expand on what you see as possible problems with this proposal? I'm interested in hearing about the Nx team's design philosophy here. For example, are we looking to limit which prompts are available in Nx? If so, is this to prevent migration issues if Nx wants to move away from enquirer in future?

I can't speak for @jskrzypek but for my team we are looking for a prompt that can generate dynamic items at run time based off of a previous prompt's answer. I was able to do this using what I had implemented since it makes it possible for us to use all options from enquirer prompts. But, as you pointed out on the PR, that only works if your prompt is defined locally.

@nzacca
Copy link

nzacca commented Sep 22, 2022

@FrozenPandaz

Also to add, I don't mind implementing this if this is not a priority right now that @AgentEnder has time for. My team is in need of this addition. If I have some direction from someone on the Nx team about how you'd like this designed api-wise and implemented, I don't mind taking a stab at it.

@brandonbechtel
Copy link

Hi, I just wanted to chime in on this and get some thoughts. I know Nx migrated over to enquirer a little over a year ago due to inquirer's dependency on RxJS and the size/performance impact. Just looking at the two repos today, it looks like inquirer is smaller (images below, though I don't know if the unpacked size metric on the npm repo page includes dependencies), but it is also being more actively developed. Is the intent of Nx to stick with enquirer?

image

image

@jskrzypek
Copy link
Author

@brandonbechtel that's a great question to ask, and I think it really deserves its own issue, since this one is regarding expansion of the supported schema-driven prompt types. After you create it, I would definitely comment back on this issue with a reference to your new one, because a decision to switch libraries would certainly impact any development work on this one.

@brandonbechtel
Copy link

Good call. I almost did that, but wanted visibility on this issue as well - to your point about it impacting this work. I ended up creating a discussion instead of an issue. You can track it here: #13902 (comment)

@jskrzypek
Copy link
Author

@nzacca @FrozenPandaz I think I would suggest/reach for a different approach to implement this. I'd opt for something a bit like what @nzacca provided via a custom prompt, but instead I'd use a JSON object to directly provide the parameters to the library's .prompt() (leaving it open whether this is enq. or inq. per @brandonbechtel's disc. above) method pretty much without translation, rather than relying on importing and evaluating code at runtime to provide that Prompt or those params. You could maybe provide some way to opt-in to that behavior, but I definitely wouldn't default to it.

The way I see it there are two different operations that end up driven by each of these param schema:

  1. Prompting the user (including collecting the input) via the library.
  2. Parsing (and thus validating) the input value returned by the library into the type/form specified on the root of the property.

If I were going to rewrite or expand how this gets handled in the future that's probably how I'd start thinking about it. Most of the current set of prompt properties/types that nx supports are definitely the most common use-cases and I'd bias for preserving the existing syntax that they use, but I may re-consider how the advanced options and specifically the x-prompt machinery is used.

The current functionality is that the property specifies how it gets parsed (i.e. operation 2.), which implies how it should be prompted (i.e. operation 1.), unless you pass an x-prompt to override that implication. This in effect means that a compatible parser needs to be specified for any allowed prompt style.

I might consider decoupling this implication by accepting a top-level "type": "custom" on properties, which would expect an x-prompt entry with arguments to pass to the prompt library without translation, and optionally an x-parse entry. If x-parse is not provided, the returned input value would be left as-is in the form the library returned it. Otherwise, x-parse would accept a) the name of a common nx type to parse the returned value into (isomorphic to just providing the same in the type), or b) arguments to pass to a parser function.

Implementation-wise I'd then refactor the existing code to translate all of the implications/shorthands into inputs to the prompt library and the parser function, making the system more extensible. This should also allow a lot of the nested, custom, or branching logic in params.ts to be abstracted away.

@tinesoft
Copy link
Contributor

Hi @jskrzypek , hi all,

Any progress on this?

I'll be interested in extending the types of prompts supported by Nx as well...

In my case, I'd like a prompt that extends the behaviour of autocomplete, while the list of choices is fetched asynchronously(via a API call fo example), something like async-autocomplete....

These kinds of extensions are not possible at the moment, with the current implementation of

type: 'input' | 'autocomplete' | 'multiselect' | 'confirm' | 'numeral';

But your suggested approach could do the trick, while remaining extensible and decoupled:

I might consider decoupling this implication by accepting a top-level "type": "custom" on properties, which would expect an x-prompt entry with arguments to pass to the prompt library without translation, and optionally an x-parse entry. If x-parse is not provided, the returned input value would be left as-is in the form the library returned it. Otherwise, x-parse would accept a) the name of a common nx type to parse the returned value into (isomorphic to just providing the same in the type), or b) arguments to pass to a parser function.

I can provide my help with the PR, if needed...

@jskrzypek
Copy link
Author

@tinesoft I no longer have time to work on this. Feel free to take it over.

@nzacca
Copy link

nzacca commented Mar 14, 2023

@tinesoft

There has been no progress on this as we've been patiently waiting for feedback from the Nx team. I submitted a PR above but that was quickly rejected since it only supported locally defined prompts. I haven't been successful in receiving any response about questions related to this task.

Just to respond to jskrzypek's 12/19 response above, a pure JSON definition won't work for my team's needs since we need prompts that can have their options dynamically generated/fetched based on a previous prompt's answer. This is possible via enquirer but only as a JS definition.

I'm more than happy to sit down (via Slack) with a member of the Nx team to discuss what they would like done here and do the work but I won't do that without feedback from the Nx team first. I don't want to keep pestering the folks on this thread. I'm just looking for some direction here before proceeding.

cc: @FrozenPandaz @AgentEnder

@AgentEnder
Copy link
Member

This is something that we are discussing internally, we'd like to support it but there are a few avenues we could take to get there. I'll remember to update this as we start to settle on a final API.

Personally, I'd like to avoid coupling any tighter to enquirer as its maintenance seems to have dropped off pretty significantly.

@nzacca
Copy link

nzacca commented Mar 14, 2023

@AgentEnder

Thank you very much for the response. Knowing that the Nx team does not want to couple to enquirer further is very helpful. I look forward to hearing news about this new API. Would it be possible to have an RFC available that the community could comment on?

@AgentEnder AgentEnder removed the community This is a good first issue for contributing label Mar 15, 2023
@kindrebelG
Copy link

My team is also interested in something like this. Similar to @nzacca, the ability to open it up for hydrating choices for a given option at runtime would be ideal. We currently do some rather convoluted & DX unfriendly schema generation to keep the options current in our generators... Would be great to nix that step.

Copy link

github-actions bot commented Dec 7, 2023

This issue has been automatically marked as stale because it hasn't had any recent activity. It will be closed in 14 days if no further activity occurs.
If we missed this issue please reply to keep it active.
Thanks for being a part of the Nx community! 🙏

@github-actions github-actions bot added the stale label Dec 7, 2023
@nzacca
Copy link

nzacca commented Dec 7, 2023

Not stale.

@github-actions github-actions bot removed the stale label Dec 8, 2023
@FrozenPandaz FrozenPandaz self-assigned this Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: core core nx functionality type: feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants