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

First item is always resolved when export map target is Array (even if non-existent) #20

Closed
helmturner opened this issue Jan 20, 2024 · 8 comments

Comments

@helmturner
Copy link

helmturner commented Jan 20, 2024

Current Behavior

Given the following exports map:

  "name": "example",
  "exports": {
    "./a": [
      "./b.js",
      "./non-existent.js"
    ],
    "./c": [
      "./non-existent.js",
      "./d.js"
    ]
  }

Where ./non-existent.js does not exist on disk, the following holds true:

import thing1 from "example/a" // imports from "example/b.js"
import thing2 from "example/c" // imports from "example/non-existent.js"

Expected Behavior

"example/c" should resolve to "example/d.js" when "./non-existent.js" is not found.

Reproduction

I've added tests that cover both cases in the associated PR. The first passes, but the second fails the strictEqual assertion.

Additional Info

I took a swing at fixing this, but no luck. The packageExportsResolve algorithm calls resolvePackageTarget when it finds a match in the exports field. resolvePackageTarget does check if target is an array, but it greedily accepts the first entry unless it throws ERR_INVALID_PACKAGE_TARGET. However, the target technically is a valid target—it just doesn't point to an existing file.

My first thought was to call fileExists(resolved) just before returning out of the loop on line 540, and, if false, continue the loop to the next item. Unfortunately that didn't seem to work for some reason.

while (++i < targetList.length) {

My next thought was that one of the other resolver functions was returning a result before the packageExportsResolve ever gets called, short-circuiting it.

Unfortunately, it seems I can't get any of my breakpoints to fire when I run it in the debugger, nor can I get any console.log calls to print to the terminal. Any ideas what might be going on here?

@wooorm
Copy link
Owner

wooorm commented Jan 20, 2024

I am under the impression that this is exactly how node works?

The code here is manually picked from node. I don't think they have that.

@helmturner
Copy link
Author

helmturner commented Jan 20, 2024

So sorry, just now realizing this code is pulled from the node algorithm, and the issue is an upstream one (and, apparently, expected behavior? 🙄)

Edit: thanks for the quick reply. This actually explains why I was having issues with another bundler—that's what got me searching for this package in the first place lol.

Edit-edit: I promise this wasn't an XY problem—I actually did (mis-)read the spec first...

@wooorm
Copy link
Owner

wooorm commented Jan 20, 2024

From what I gather, the “spec” was I think just a random idea years ago. Then folks implemented it. And figured out that it was super complex to do everything. And decided to rip things out.

Personally, I feel like conditions do a good job already at pointing to one file or another. And whether files currently exist or not isn’t something that should be related to the public API of a package.

You may like https://github.com/wooorm/package-exports, which attempt to lint package exports!

@helmturner
Copy link
Author

Nice! That would have saved me a headache. Although I noticed the rule that would have caught this says 'Many tools don't support this... Heh, including node!

I reckon most folks want this for simulating the 'CJS' semantics of guessing extensions, but this actually has a valid use-case in a cross-platform environment. I left my 2 cents on their issue, but I'll just have to go back to the drawing board on my end 🤷

@wooorm
Copy link
Owner

wooorm commented Jan 20, 2024

ok, well a) don’t restore extension guessing, b) you’re talking about “conditionally resolve”, and that’s something that conditions already do? c) you can use asterisks as well.

"./prefix-*-suffix": {
  "native": "./path/to/folder/*.native.js",
  "default": "./path/to/folder/*.default.js"
}

(prefix and suffix here just an example. depends on your actual files what values to use)

@wooorm
Copy link
Owner

wooorm commented Jan 20, 2024

This sounds more like you have a question on how best to expose your package’s API. That’s fine. But then I need you to provide me the input files. Then I can help you.

@helmturner
Copy link
Author

helmturner commented Jan 21, 2024

I appreciate your offer to help, especially considering I didn't even open an issue in the right repo 😅

I think I know an approach, but it's not elegant. There's really not a good way to achieve what I'm aiming for.

I've got two apps which share a multiple cross-platform internal packages. One, the UI library, is compiled before consumption. In effect, this just means that all built files end with .js—however there are still .native.js and/or .web.js files with the same name. I'll focus on the non-transpiled modules, because it includes the additional complexity of having both tsx vs ts files (in addition to the platform extensions)

Many of the package modules are pure react, using only platform-agnostic APIs. For these files, I would like just to have a single .ts or .tsx module that resolves the same on both platforms.

Some of the modules, however, have unique implementations for each platform. For these, I would like to have *.native.* files only resolved by Metro, and *.web.* files only resolved by webpack. A third version, without the platform extension, exports a type that (through some type magic) is different depending on the importing package's configuration.

Setting aside the wisdom of extension guessing, this is the exports map I hoped to use:

"exports": {
    ".": {
      "types": [
        "./src/index.ts",
        "./src/index.tsx"
      ],
      "browser": [
        "./src/index.web.ts",
        "./src/index.web.tsx",
        "./src/index.ts",
        "./src/index.tsx"
      ],
      "ios": [
        "./src/index.ios.ts",
        "./src/index.ios.tsx",
        "./src/index.ts",
        "./src/index.tsx"
      ],
      "android": [
        "./src/index.android.ts",
        "./src/index.android.tsx",
        "./src/index.ts",
        "./src/index.tsx"
      ],
      "native": [
        "./src/index.native.ts",
        "./src/index.native.tsx",
        "./src/index.ts",
        "./src/index.tsx"
      ],
      "default": [
        "./src/index.ts",
        "./src/index.tsx"
      ]
    },
    "./*": {
      "types": [
        "./src/*.ts",
        "./src/*/index.ts",
        "./src/*.tsx",
        "./src/*/index.tsx"
      ],
      "browser": [
        "./src/*.web.ts",
        "./src/*/index.web.ts",
        "./src/*.web.tsx",
        "./src/*/index.web.tsx",
        "./src/*.ts",
        "./src/*/index.ts",
        "./src/*.tsx",
        "./src/*/index.tsx"
      ],
      "ios": [
        "./src/*.ios.ts",
        "./src/*/index.ios.ts",
        "./src/*.ios.tsx",
        "./src/*/index.ios.tsx",
        "./src/*.ts",
        "./src/*/index.ts",
        "./src/*.tsx",
        "./src/*/index.tsx"
      ],
      "android": [
        "./src/*.android.ts",
        "./src/*/index.android.ts",
        "./src/*.android.tsx",
        "./src/*/index.android.tsx",
        "./src/*.ts",
        "./src/*/index.ts",
        "./src/*.tsx",
        "./src/*/index.tsx"
      ],
      "native": [
        "./src/*.native.ts",
        "./src/*/index.native.ts",
        "./src/*.native.tsx",
        "./src/*/index.native.tsx",
        "./src/*.ts",
        "./src/*/index.ts",
        "./src/*.tsx",
        "./src/*/index.tsx"
      ],
      "default": [
        "./src/*.ts",
        "./src/*/index.ts",
        "./src/*.tsx",
        "./src/*/index.tsx"
      ]
    }
}

and here is an example of the file tree:

packages/app/src
├── providers.tsx # Should be resolved the same regardless of platform, including types
├── fonts
│   ├── Inter.web.ts # Should be exclusively resolved by Webpack
│  ├── Inter.native.ts # Should be exclusively resolved by Metro
│  └── Inter.ts # Should be exclusively resolved by Typescript
└── index.ts # Should be resolved the same regardless of platform, including types

Now, TypeScript is smart enough to know that if I import *.js and there's a matching *.ts or *.tsx, then that's the right target... but I think that only helps if I transpile the package first, not if it is bundled by the consumer.

Unless you see something I'm missing, the only option is to 1) transpile all packages and 2) explicitly export all modules with a .native/.web split. Well, the only option without writing a custom resolution plugin for every bundler...

EDIT: To add an additional wrinkle, Metro Bundler does not respect platform extensions when using the exports field. So I may have no option but to ditch it for main exports...

@wooorm
Copy link
Owner

wooorm commented Jan 21, 2024

Can you post all the files you have in src/, and in dest/?
This is all very theoretical currently.

I would recommend taking some time to think about what you want to expose.
Similar to how you don’t export every variable from a file, not every file is a public interface.

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.

2 participants