Revert "readline: add stricter validation for functions called after close

[dario-piotrowicz]

This reverts commit 8e7f32f (PR #57680) since the PR is introducing an unexpected issue in the REPL, see: #58014 (comment)

Fixes #58009

---

Thanks a lot @lpinca for spotting the problem 🙏

Once again I'm super sorry for the inconvenience here, I'm really surprised that such an issue didn't cause any test failures 😓

I'm try to reintroduce the changes in #57680 later making sure not to break the REPL 🙇

[BridgeAR]

Should we not try to address the shown issue instead?
I have the feeling the failures here are exactly what should be achieved?

[Edit] I see that the validations should be reintroduced later again. I guess that's also fine if it takes too long to fix the issues.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/66458/

[lpinca]

[Edit] I see that the validations should be reintroduced later again. I guess that's also fine if it takes too long to fix the issues.

Yes, the added benefit is the immediate "fix" of the flaky test (#58009).

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.27%. Comparing base (b665127) to head (2f84449).
Report is 7 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #58024   +/-   ##
=======================================
  Coverage   90.27%   90.27%           
=======================================
  Files         630      630           
  Lines      186158   186149    -9     
  Branches    36475    36477    +2     
=======================================
+ Hits       168047   168054    +7     
+ Misses      10980    10978    -2     
+ Partials     7131     7117   -14     

Files with missing lines	Coverage Δ
lib/internal/readline/interface.js	96.79% <ø> (-0.02%)	⬇️

... and 26 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Fast-track has been requested by @aduh95. Please 👍 to approve.

[dario-piotrowicz]

Should we not try to address the shown issue instead?

I'm very happy to look into the underlying issue (and I will) however unfortunately I'm visiting family for easter (I'm back home early next week) so I currently don't have as much time as I'd otherwise have to look into this promptly 🥲

So instead of keeping the bug around and cause issues maybe it'd be safer/quicker to just revert the change
(if this is not too urgent and there's time to breathe I'm happy not to revert the change)

I have the feeling the failures here are exactly what should be achieved?

Not really, the issue here is that this is breaking valid usage of the REPL 😓 (see #58014 (comment))

[Edit] I see that the validations should be reintroduced later again. I guess that's also fine if it takes too long to fix the issues.

Again I feel like this might be the most convenient solution but I am happy to proceed in any way people feel it's more appropriate here 🙂 (and again I'm very sorry for the inconvenience 🙇😢)

[nodejs-github-bot]

Landed in b673c69

[RafaelGSS]

It seems this change is required or the v24 proposal fails @richardlau

[richardlau]

It seems this change is required or the v24 proposal fails @richardlau

@RafaelGSS If this is about the dont-land-on-v24.x label, then I added that when #57680 was not going to be included in 24.0.0 (as it was semver major and past the cut-off date for 24.0.0).