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

[request] separate glob-cli from glob #542

Closed
alumni opened this issue Jul 25, 2023 · 3 comments
Closed

[request] separate glob-cli from glob #542

alumni opened this issue Jul 25, 2023 · 3 comments

Comments

@alumni
Copy link

alumni commented Jul 25, 2023

I was investigating CommonJS <-> ESM interop issues on yarn v1 and found the CJS dependencies installed by glob > jackspeak > @isaacs/cliui as the cause of this. See this comment too: yargs/cliui#139 (comment)

TBH, I had no idea that glob has its own built-in CLI, and my question is now why? We use glob rather as a library than as an executable.

I would suggest extracting the CLI into a separate package.

@isaacs
Copy link
Owner

isaacs commented Jul 25, 2023

Yarn allegedly supports package aliases, but resolves deps incorrectly on update. This is a yarn bug. Use npm. It works. Or, blow away your yarn.lock and do a fresh install if you're really attached to yarn, as that seems to be a workaround people have found.

I'm not going to work around yarn bugs in my libraries when pnpm and npm can handle it just fine, sorry.

@isaacs isaacs closed this as not planned Won't fix, can't repro, duplicate, stale Jul 25, 2023
@alumni
Copy link
Author

alumni commented Jul 26, 2023

Removing yarn.lock and fresh install doesn't work.

Manually editing yarn.lock and adding name: string-width to the package would work, however I do not want to do that since the file is auto-generated.

Resolutions work fine with yarn, one could set jackspeak to resolve to a non-existing local path, or switch to the original cliui:

{
    "resolutions": {
        "jackspeak": "./node_modules/.cache/null",
        //  or:
        "@isaacs/cliui": "npm:cliui@8.0.1",
    }
}

This would do the trick, however, since glob is a widely used package, it would be nice if we did not have to work around it, especially since the feature that causes the issue is not really a core feature and brings in a lot of other dependencies :)

@isaacs
Copy link
Owner

isaacs commented Jul 26, 2023

I'm sorry to hear that deleting the yarn lock and reinstalling didn't fix the issue. I thought someone else had gotten it to work that way, I'll try to dig up the older issue when I'm at a full keyboard, maybe they have some other steps that they could share.

The added deps are tiny. If they were vendored, or if yarn didn't have this bug, you wouldn't notice the added disk usage. And it's only "not a core feature" from the point of view of someone who doesn't use it. For those who do, it's a reason they use this package.

I'm not going to stop using dependency aliasing when it's useful. As more parts of our ecosystem go esm-only, it's likely that those of us writing hybrid packages will need to use that workaround again in the future. And, I use jackspeak in every cli I write these days. So, it is almost certainly going to bite you again eventually.

In fact, it may have already, in a more insidious way; if the aliased deps in question were not esm-only, thus causing a crash when loaded, it's likely that your programs already just get the wrong versions of aliased deps, which can cause incorrect behavior in ways that are not as easy to detect early.

Your package manager is failing to resolve dependency contracts correctly, and putting the work on you to define resolutions manually. Use a package manager that can manage packages. I'm not trying to be harsh, but having spent over a decade working on npm, it's somewhat shocking to me that such an obvious bug has persisted for so long.

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

No branches or pull requests

2 participants