-
Notifications
You must be signed in to change notification settings - Fork 530
fix(node): make readline Interface and Readline classes no-op stubs #6016
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
base: main
Are you sure you want to change the base?
Conversation
Change the readline Interface class and the readline/promises Interface and Readline classes to be no-op stubs instead of throwing errors. This matches the behavior of unenv's readline implementation, which allows code that depends on readline to work without crashing even though the functionality is not fully implemented. Methods now return sensible defaults (empty strings, zero values, etc.) instead of throwing ERR_METHOD_NOT_IMPLEMENTED errors. Fixes compatibility with cloudflare/workers-sdk#11734
|
The generated output of |
| throw new ERR_METHOD_NOT_IMPLEMENTED('Interface.question'); | ||
| // If callback is the second argument (no options) | ||
| if (typeof _options === 'function') { | ||
| (_options as (answer: string) => void)(''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you remove unknown type in function declaration and use the union of two types to avoid this?
| [Symbol.asyncIterator](): NodeJS.AsyncIterator<string, undefined, any> { | ||
| throw new ERR_METHOD_NOT_IMPLEMENTED('Interface[Symbol.asyncIterator]'); | ||
| async *[Symbol.asyncIterator](): NodeJS.AsyncIterator<string> { | ||
| yield ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a comment to why we are returning this?
| [Symbol.asyncIterator](): NodeJS.AsyncIterator<string, undefined, any> { | ||
| throw new ERR_METHOD_NOT_IMPLEMENTED('Interface[Symbol.asyncIterator]'); | ||
| async *[Symbol.asyncIterator](): NodeJS.AsyncIterator<string> { | ||
| yield ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a small comment?
Summary
Interfaceclass and the readline/promisesInterfaceandReadlineclasses to be no-op stubs instead of throwing errorsERR_METHOD_NOT_IMPLEMENTEDerrorsFixes compatibility with cloudflare/workers-sdk#11734