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

Version 9.3.2 released to npm does not work #1456

Closed
Choppel opened this issue Jul 2, 2024 · 11 comments · Fixed by #1457
Closed

Version 9.3.2 released to npm does not work #1456

Choppel opened this issue Jul 2, 2024 · 11 comments · Fixed by #1457

Comments

@Choppel
Copy link

Choppel commented Jul 2, 2024

As stated in the title, version 9.3.2 released to npm does not work properly. An import like import Separator from 'inquirer/lib/objects/separator.js'; which worked prior to 9.3.2 now throws ERR_MODULE_NOT_FOUND.

Weirdly enough, I couldn't find this version on the releases page of GitHub.

@SBoudrias
Copy link
Owner

Ohh, I didn't expect people to rely on the internal files structure to the package.

You should import { Separator } from 'inquirer' and rely on documented interface.

@Choppel
Copy link
Author

Choppel commented Jul 2, 2024

Did you test this?

import { Separator } from 'inquirer' throws TS2614: Module 'inquirer' has no exported member Separator. Did you mean to use import Separator from "inquirer" instead? for me.

@SBoudrias
Copy link
Owner

We do have integration tests for some of the exports, though not the Separator. This is the legacy package I don't work with too often. I'll take a look, thanks for the report.

@SBoudrias SBoudrias reopened this Jul 2, 2024
@SBoudrias
Copy link
Owner

Alright, so went to verify and the documented syntax is as follow:

import inquirer from 'inquirer';

new inquirer.Separator();

I double-checked the documentation and direct file access isn't documented. So while I agree it's 9.3.2 comes with an implicit breaking change, I don't think it's a reasonable expectation that the folder/file structure from a project to not change. Node project shouldn't import from undocumented direct file access to package internals.

I'm open to revisiting this issue if multiple users run into this issue somehow; so please post below if you've also encountered this issue. But in all cases, a future safe solution is to rely on inquirer.Separator.

@Choppel
Copy link
Author

Choppel commented Jul 3, 2024

Hello @SBoudrias,
this works now. Thanks.

I guess other users are also running into this issue. Because inquirer-autocomplete-prompt does not work anymore with [email protected] seemingly because of the changed project structure. See mokkabonna/inquirer-autocomplete-prompt#159

But maybe it's the way of importing is also a misconception by the creator of the plugin. I am going to let them know.

@SBoudrias
Copy link
Owner

Ohhh, that's a bit more of an issue. There's so many plugins and they might access internals... Ugh, I'll work out a solution to maintain that (maybe defining an exports field in the package.json)

@SBoudrias SBoudrias reopened this Jul 3, 2024
SBoudrias added a commit that referenced this issue Jul 3, 2024
…s. (#1457)

* Chore(turbo): Consider root tsconfig.json as a dependency

* Fix(inquirer): Fix changed paths from internals used by plugin vendors. Fix #1456
@SBoudrias
Copy link
Owner

[email protected] is out and should fix all this. Thanks for the report!

@Choppel
Copy link
Author

Choppel commented Jul 4, 2024

Works. Thanks for the quick reply.

@deot
Copy link

deot commented Jul 4, 2024

This seems a bit bloated. Would it be better to notify other authors to modify it in the next major version?

@SBoudrias
Copy link
Owner

It would, but that's the unfortunate reality of old projects (>10 years old) with large adoption. Breaking change risk leaving large chunks of the ecosystem behind.

But anyhow, the true fix is for plugins to migrate to @inquirer/core. I'm just trying to smooth the migration away with inquirer.

@Choppel
Copy link
Author

Choppel commented Jul 5, 2024

Oh. I didn't notice that you are rewriting inquirer. Maybe add a notice to the legacy inquirer project during install/update.
I am going to rewrite my code based on the new @inquirer classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants