sindresorhus / globby

User-friendly glob matching
MIT License
2.49k stars 126 forks source link

`toPath` imported from `unicorn-magic` is not exported by the `0.1.0` dependency of that package in non-"node" context causing Webpack failures #260

Open DavidAnson opened 8 months ago

DavidAnson commented 8 months ago

This change introduces failures compiling globby under Webpack with target=webworker using a resolve/fallback for fileURLToPath in the url module.

I'm sure that scenario sounds weird, but it used to work with a certain amount of gymnastics: https://github.com/DavidAnson/markdownlint-cli2/tree/main/webworker

This commit introduces the issue by hiding a call to fileURLToPath inside the Node-only export toPath: https://github.com/sindresorhus/globby/commit/2c06ae52cd46a9c85dd9029023c02165ee40369d#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519

Unfortunately, I may be the only person on the planet who cares? FYI, I guess. I'll try more gymnastics to get my scenario working again.

falcosan commented 8 months ago

@DavidAnson you are not alone! I use this package because Nuxt uses globby as a dependency. However, even when using Vite to compile my Nuxt project, I still encounter the same error.

ERROR:

✘ [ERROR] No matching export in "node_modules/unicorn-magic/default.js" for import "toPath"

    node_modules/globby/ignore.js:8:8:
      8 │ import {toPath} from 'unicorn-magic';
        ╵         ~~~~~~

✘ [ERROR] No matching export in "node_modules/unicorn-magic/default.js" for import "toPath"

    node_modules/globby/index.js:7:8:
      7 │ import {toPath} from 'unicorn-magic';
DavidAnson commented 8 months ago

Bummer. In case it helps, here's my workaround (ignore the setImmediate changes): https://github.com/DavidAnson/markdownlint-cli2/commit/a37b78002020ae953c62cc3ff604c161fcf669b2

boxexchanger commented 7 months ago

In the new version of globby 14.0.0, the following issue has been identified:

In the file globby/ignore.js, there is an import statement:

import {toPath} from 'unicorn-magic';

This import leads to a submodule where the file unicorn-magic/default.js is imported. From the context, it seems that the intention was to import unicorn-magic/node.js, but the actual import is missing.

Due to the absence of the export for toPath, we encounter the following error:

"toPath" is not exported by "node_modules/unicorn-magic/default.js", imported by "node_modules/globby/ignore.js".

We kindly request you to address this issue promptly. One potential solution is to move the toPath function into the default.js file for proper export. Thank you for your attention to this matter, and we look forward to a swift resolution.

boxexchanger commented 7 months ago

for hot fix module i've created postinstall script until we wait fix from @sindresorhus

import fs from 'node:fs';

const text = fs.readFileSync('./node_modules/unicorn-magic/default.js', 'utf8');
const newScript = `import {fileURLToPath} from 'node:url';
export function toPath(urlOrPath) {
\treturn urlOrPath instanceof URL ? fileURLToPath(urlOrPath) : urlOrPath;
}
${text}`;

if(text.indexOf("toPath(") === -1) {
  console.log("Fixing unicorn-magic script...")
  fs.writeFileSync('./node_modules/unicorn-magic/default.js', newScript, 'utf8');
}else{
  console.log("Skip fix unicorn-magic script.");
}
AlexAegis commented 7 months ago

I also encountered this

Kjir commented 7 months ago

If, like me, you are using vite and the problem appears in a dependency on @nuxt/kit, here is a way to fix the issue. Update your vite.config.js and add:

    optimizeDeps: {
      exclude: ["@nuxt/kit"],
    }

Further details are explained here: https://github.com/storyblok/storyblok-nuxt/pull/662

AlexAegis commented 4 months ago

I had a little time to look into this problem further.

For context: I'm using globby in a github action and I have to bundle it into a single file so it can run immediately after the action is checked out. Here there's no node_modules the only imports available are from node.

What's the problem?

unicorn-magic is using conditional exports to separate node only capabilities and to allow this package to be used in non-node environments too. There's nothing wrong with this, this is the purpose of the 'node' conditional.

unicorn-magic/package.json

    "exports": {
        "types": "./index.d.ts",
        "node": "./node.js",
        "default": "./default.js"
    },

This is technically not perfect though as typescript will use the same type definitions for both conditions, publint even warns for this.

The problem we are facing is that when globby and unicorn-magic gets bundled the bundler doesn't trigger the node export condition.

You have two options.

The dirty solution: replacing the unicorn-magic module with a vendored version

I'm using vite but there's a webpack example above too

src/unicorn-magic.js

import { fileURLToPath } from 'node:url';

export function toPath(urlOrPath) {
    return urlOrPath instanceof URL ? fileURLToPath(urlOrPath) : urlOrPath;
}

Then in the vite config:

...
    resolve: {
        alias: {
            'unicorn-magic': 'src/unicorn-magic.js'
        }
    },
...

This is a quick and dirty solution and I do not recommend it because as soon as Sindre will use an updated version in globby, it will break.

The proper solution: Tell the bundler to use the node conditional

This is something that dependending on your bundler you may not be able to do, since it requires explicit support from the bundler. And I'm pretty sure vite doesn't use the node condition every time it should. But it does when ssr is on so I was able to use that:

export default defineConfig({
    build: {
        target: 'node20',
        ssr: true,
        lib: {
            entry: '',
            formats: ['es'],
        },
        rollupOptions: {
            external: (source: string): boolean => {
                return builtinModules.includes(source) || source.startsWith('node:');
            },
        },
    },
    ssr: {
        target: 'node',
        noExternal: ['@actions/core', '@actions/exec', '@alexaegis/workspace-tools', 'globby'],
    },
});

A drawback here is that for the ssr specific external settings vite doesn't have a nice callback option like in rollupOptions so you have to do some additional work, either manually listing all your dependencies that you want to bundle in (an ssr bundle assumes node_modules will be available, for me that's not an option) or write something that does this for you.

The nice solution (as a package author)

I bet that there are cases where to achieve multi-platform support these export conditions are necessary, but when it's not IMHO it's simpler to just have separate export paths (unicorn-magic/node) for the platform specific stuff, that too would solve this problem.

    "exports": {
        ".": {
            "types": "./index.d.ts",
            "default": "./default.js"
        },
        "./node": {
            "types": "./index.d.ts",
            "default": "./node.js"
        }
    },

Ultimately, the blame is not on globby or unicorn-magic but on the bundlers and the configs we write for them, but at the same time, posing less challenges is a nice gesture.