feat(inquirer): implement createPromptSession #1557
Conversation
mshima
changed the title
mshima
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1557 +/- ##
=======================================
Coverage ? 98.07%
=======================================
Files ? 39
Lines ? 2442
Branches ? 651
=======================================
Hits ? 2395
Misses ? 40
Partials ? 7 ☔ View full report in Codecov by Sentry. |
mshima
force-pushed
the
subject
branch
3 times, most recently
from
1d34dd5
to
8927649
Compare
packages/inquirer/src/ui/prompt.mts
Outdated
| question.askAnswered !== true && | ||
| _.get(this.answers, question.name) !== undefined | ||
| ) { | ||
| if (question.askAnswered !== true && this.answers[question.name] !== undefined) { |
There was a problem hiding this comment.
This will break where name contains a sub-path like name: 'foo.bar' (which is why _.get was used)
There was a problem hiding this comment.
It's using a proxy.
Tests are in place for sub-paths.
There was a problem hiding this comment.
Oh, I missed that. I think it's fair to do internally, but I'm unclear why we want to expose the capacity to provide a proxy as argument?
There was a problem hiding this comment.
I want to provide a custom store.
It's possible to use a proxy or implement an api.
This is actually required to simplify supporting untouched inquirer api.
So Observables are supported by loading the question answer on demand.
Currently the prefilled answers are built from questions array.
There was a problem hiding this comment.
This is a feature request, I can drop it.
There was a problem hiding this comment.
I'm not against it if there's a use-case. Just want to make sure we don't add unnecessary complexity we then need to support in "perpetuity". Or open doors for people to unnecessarily overcomplexify their implementations.
|
I like the concept/interface, I'd really prefer to eventually move out |
Related to #1527.