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

Support package.json exports "Alternatives" #5052

Closed
puehringer opened this issue Dec 18, 2023 · 18 comments
Closed

Support package.json exports "Alternatives" #5052

puehringer opened this issue Dec 18, 2023 · 18 comments
Labels
A-resolver Area: resolver feat New feature or request

Comments

@puehringer
Copy link

puehringer commented Dec 18, 2023

What problem does this feature solve?

webpack supports Alternatives in package.json#exports fields: https://webpack.js.org/guides/package-exports/#alternatives

This allows us to import from one path, if possible, and fallback to another path if required. Currently in rspack, this will fail with a Failed to resolve in case the first occurance (./dist/index.js) does not exist. It works in webpack however.

  "exports": {
    ".": {
      "types": [
        "./dist/index.d.ts",
        "./src/index.ts"
      ],
      "import": [
        "./dist/index.js",
        "./src/index.ts"
      ]
    }
  }

What does the proposed API of configuration look like?

No API required.

@puehringer puehringer added feat New feature or request pending triage The issue/PR is currently untouched. labels Dec 18, 2023
@hardfist hardfist added the A-resolver Area: resolver label Dec 18, 2023
@Boshen
Copy link
Contributor

Boshen commented Dec 18, 2023

Do you have a real package json with real use case for me to test on?

@puehringer
Copy link
Author

puehringer commented Dec 18, 2023

Hi @Boshen, I can't make our repository available, but I created a minimal reproducible demo: rspack_export_alternatives.zip

Instructions are in the README.md (and pasted here):

lib contains the library to install, with exports pointing to dist and src.

app is consuming this library (installed via portal).

To reproduce, do the following:

cd app
yarn
yarn build

This will create a app/dist/main.js with content Hello from dist.

Now, remove the lib/dist/lib.js file. webpack would now fallback to the lib/src/lib.ts file, but rspack fails with "Failed to resolve lib in ...".

Let me know if you need anything else, and thanks for getting back to me so quickly!

@Boshen
Copy link
Contributor

Boshen commented Dec 19, 2023

I know this day will come when I was implementing this feature, but not this soon - because I found a discrepancy / compatibility issue between the implementations between webpack/enhanced-resolve and node.js ESM.

This is going to be a long explanation detailing what's going on with this requirement, so please bear with me.


Let's start with the X/Y problem:

The Y problem

What you are actually trying to do is:

  • bundle the source .ts code during development
  • bundle the js code when it's shipped to your private registry for others to consume

This situation comes from the fact that we are importing the package inside our monorepo via workspace: * or portal from the repro above. The transpiled .js file is obviously not there for the imported package in our development workflow, so we resort to ways that can import the .ts file and bundle from source instead.

I've seen this situation at ByteDance for all of our monorepos, and the solution we have settled on is using tsconfig paths + changing the resolve.extensions option to accept the .ts extention (the default in Rspack). This is a solution asked by the community and is offered in the tsconfig-paths-webpack-plugin.

Rspack (oxc_resolver as the module resolver) has provided built-in support for this feature as documented here, we even provided builtin support for TypeScript project references otherwise path aliases inside these imported .ts sources will not work.

The X Problem

What should be the correct behavior for exports array (alternatives)?

Given the module /node_modules/foo with the following package.json:

{
  "name": "foo",
  "exports": {
     "./bar": ["bad-specifier", "./non-existent.js", "./existent.js"]
  }
}

and require("foo/bar"):

webpack/enhanced-resolve returns "/node_modules/foo/existent.js".

node.js returns Error: Cannot find module '/node_modules/foo/non-existent.js'.

The node.js behavior is backed by the specification stated in Resolution Algorithm Specification:

PACKAGE_TARGET_RESOLVE(packageURL, target, patternMatch, isImports, conditions)
...
3. Otherwise, if target is an Array, then
...
  2. For each item targetValue in target, do
    1. Let resolved be the result of PACKAGE_TARGET_RESOLVE( packageURL, targetValue, patternMatch, isImports, conditions), continuing the loop on any Invalid Package Target error.

Notice the statement continuing the loop on any Invalid Package Target error, it is not "Not Found Error".

As for webpack, the documentation states the following:

Instead of providing a single result, the package author may provide a list of results. In such a scenario this list is tried in order and the first valid result will be used.

It is hinting that it will try the values in order not matter what the error is, and return the successful one. This is backed by testing the enhanced-resolve implementation.


I don't know how to deal with this situation so I'm going to call the experts here.

Best Regards,

Boshen.

@Boshen
Copy link
Contributor

Boshen commented Dec 19, 2023

Andrew replied:

As for loading TS files in development, I use conditional exports for that https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/3f5e05f5a40ef117874430cb226b0088ee7233ae/packages/core/package.json#L29

@hardfist
Copy link
Contributor

nodejs/node#37928 (comment) It's a known issue in Node.js side and It seems Node.js current behavior is not useful for real world application and Webpack and Typescript all choose fallback style, so It seems following enhanced-resolve's way is better for Rspack.
@andrewbranch But I'm not sure Typescript choose fallback style is a feature or bug?

@hardfist hardfist removed the pending triage The issue/PR is currently untouched. label Dec 19, 2023
@andrewbranch
Copy link

For TypeScript, it’s a bug when it happens in --moduleResolution nodenext (microsoft/TypeScript#50762). If you’re in --moduleResolution bundler, I guess it’s a feature 😄

@hardfist
Copy link
Contributor

For TypeScript, it’s a bug when it happens in --moduleResolution nodenext (microsoft/TypeScript#50762). If you’re in --moduleResolution bundler, I guess it’s a feature 😄

that is interesting,@alexander-akait sorry to bother you,I am also not sure whether webpack current fallback strategy is a feature or bug,or is aligning resolve algorithm with Node.js is a goal of enhanced-resolve?

@alexander-akait
Copy link

alexander-akait commented Dec 20, 2023

That's intresting, especial nodejs/node#37928 (comment), I want to say it is not a bug, the fallback style is useful and provide more power

  1. Let resolved be the result of PACKAGE_TARGET_RESOLVE( packageURL, targetValue, patternMatch, isImports, conditions), continuing the loop on any Invalid Package Target error.

We literally read it as - if you get an error then continue

nodejs/node#37928 (comment)

Exports is generally designed to resolve unambiguously without hitting the disk. So what you're observing is working as intended: As soon as we find a valid file name, we assume that it's the intended target. It explicitly doesn't matter if the file currently exists or not. E.g. it would be valid to resolve to a non-existing path, create the file, and then load it.

I disagree with that, because what's the point using an array of alternatives if we will take the first valid target.

I think this sentence explains the logic pretty clearly (nodejs/node#37928):

Fallback arrays allow validation failures to continue checking the next item in the fallback array providing forwards compatiblitiy for new features in future based on extending these validation rules to new cases.

@hardfist
Copy link
Contributor

hardfist commented Dec 21, 2023

@alexander-akait thanks for your detail explanation, @Boshen since Typescript(moduleResolution set to bundler) and Webpack have the same behavior(choose fallback), It seems we should align this behavior with enhanced-resolve other than Node.js current behavior.

@Boshen
Copy link
Contributor

Boshen commented Dec 22, 2023

Given this twitter thread and my understanding of future compatibility, my advice is also do not use export arrays.

Remember, packages are published to public / private npm registries, anything that do not conform to the specification will break future compatibility. It does not matter whether the given feature is useful or not.

As for alternative solutions to what I believe what the actual problem is, we have:

  1. use tsconfig paths alias
  2. use conditional exports https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/3f5e05f5a40ef117874430cb226b0088ee7233ae/packages/core/package.json#L29

@alexander-akait
Copy link

It might be useful to ping someone from the Node.js team here because I want to hear the real reason why it was implemented this way

Copy link

stale bot commented Feb 20, 2024

This issue has been automatically marked as stale because it has not had recent activity. If this issue is still affecting you, please leave any comment (for example, "bump"). We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@Boshen
Copy link
Contributor

Boshen commented Mar 29, 2024

Closing due to lack of actionable items.

Please see all the linked issues in bluwy/publint#92

@Boshen Boshen closed this as not planned Won't fix, can't repro, duplicate, stale Mar 29, 2024
@yf-yang
Copy link

yf-yang commented Jun 14, 2024

@Boshen
Hi, I tried to follow your advice on the monorepo, but I can't get it to work. Would you mind taking a look and explaining what to do in detail?

the solution we have settled on is using tsconfig paths + changing the resolve.extensions option to accept the .ts extention (the default in Rspack)

reproduction:
https://github.com/yf-yang/modern-js-examples/tree/resolve-ts

Brief:

  • There is one modernjs app repo and a dep repo. pnpm is the package manager and it uses "dep": "workspace:^" as the dependency.
  • Project reference is set in app's tsconfig.
  • If set "exports": "src/index.ts" in dep's package.json, it works.
  • If remove "exports": "src/index.ts" in dep's package.json, then configure app's modern.config.js:
tools: {
    rspack: config => {
      config.resolve = {
        ...(config.resolve ?? {}),
        tsConfig: {
          configFile: path.resolve(__dirname, './tsconfig.json'),
          references: 'auto',
        },
      };
      return config;
    }
}

Run

cd examples/basic-app/
pnpm start

Then the dev server fails to resolve the dependency.

I think I missed some points here. Thank you for your advice!

@Boshen
Copy link
Contributor

Boshen commented Jun 14, 2024

@yf-yang Can you create a new issue with the expected outcome? This issue shouldn't be used to triage your issue.

@yf-yang
Copy link

yf-yang commented Jun 14, 2024

Sure
I just found a doc in modernjs
https://modernjs.dev/en/guides/advanced-features/source-build.html

I'll give it a try and then create another issue if stuck. Thank you for your attention

@hardfist hardfist reopened this Jun 15, 2024
@stale stale bot removed the stale label Jun 15, 2024
@hardfist
Copy link
Contributor

@yf-yang we are reconsidering support this feature

@hardfist
Copy link
Contributor

hardfist commented Aug 13, 2024

since webpack deprecate alternative in webpack/enhanced-resolve#429, Rspack's current behavior is same as latest webpack

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-resolver Area: resolver feat New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants