oven-sh / bun

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one
https://bun.sh
Other
74.21k stars 2.77k forks source link

issues with package.json exports resolution #15053

Open panva opened 4 days ago

panva commented 4 days ago

Refs: https://github.com/wintercg/runtime-keys/issues/5 Refs: https://github.com/wintercg/runtime-keys/issues/18

One can easily believe that the following exports condition examples mean "everything but node loads from the webapi dist".

"exports": {
    "node": "./dist/node/index.js",
    "bun": "./dist/bun/index.js"
},
"exports": {
    "node": "./dist/node/index.js",
    "default": "./dist/webapi/index.js"
},

Unfortunately Bun will load the "node" condition in both examples above despite there being a default or, even worse offense, an explicit "bun" one.

panva commented 4 days ago

Bottom line: As a module author I want to be able to write a module that exports "for node" and "for everything else" with two simple package.json exports. However the non-Node runtime's behaviour are in the way of being able to do that.

RiskyMH commented 4 days ago

"Whichever one of these conditions occurs first in the package.json is used to determine the package's entrypoint." https://bun.sh/docs/runtime/modules#importing-packages

My understanding is you should put the "bun" condition above node and then bun will prefer it. However, I do agree it can get confusing sometimes and if bun just looked for its condition first then it could make more sense.

guest271314 commented 3 days ago

Bottom line: As a module author I want to be able to write a module that exports "for node" and "for everything else" with two simple exports.

JavaScript runtimes that have adopted some of WinterCG have navigator.userAgent.

This is what I do, in JavaScript, for my JavaScript runtime agnostic scripts. A simple if condition, evaluating navigator.userAgent.

I am not going to be writing separate complete scripts just for a single runtime. We don't even need a Node.js-specific package.json file for Node.js to run Ecmascript Modules now that node detects Ecmascript Module syntax and has --experimental-default-type=module https://github.com/guest271314/NativeMessagingHosts/blob/main/nm_host.js#L7C1-L42C2.

const runtime = navigator.userAgent;
const buffer = new ArrayBuffer(0, { maxByteLength: 1024 ** 2 });
const view = new DataView(buffer);
const encoder = new TextEncoder();
// const { dirname, filename, url } = import.meta;

let readable, writable, exit; // args

if (runtime.startsWith("Deno")) {
  ({ readable } = Deno.stdin);
  ({ writable } = Deno.stdout);
  ({ exit } = Deno);
  // ({ args } = Deno);
}

if (runtime.startsWith("Node")) {
  readable = process.stdin;
  writable = new WritableStream({
    write(value) {
       process.stdout.write(value);
    }
  });
  ({ exit } = process);
  // ({ argv: args } = process);
}

if (runtime.startsWith("Bun")) {
  readable = Bun.file("/dev/stdin").stream();
  writable = new WritableStream({
    async write(value) {
      await Bun.write(Bun.stdout, value);
    },
  }, new CountQueuingStrategy({ highWaterMark: Infinity }));
  ({ exit } = process);
  // ({ argv: args } = Bun);
}

Note: Microsoft TypeScript version of the same code will make tsc (Nightly) throw, because tsc can't handle both Bun and Node.js declaring fetch.

import process from "node:process";
const runtime: string = navigator.userAgent;
const buffer: ArrayBuffer = new ArrayBuffer(0, { maxByteLength: 1024 ** 2 });
const view: DataView = new DataView(buffer);
const encoder: TextEncoder = new TextEncoder();

let readable: NodeJS.ReadStream & { fd: 0 } | ReadableStream<Uint8Array>,
  writable: WritableStream<Uint8Array>,
  exit: () => void = () => {};

if (runtime.startsWith("Deno")) {
  ({ readable } = Deno.stdin);
  ({ writable } = Deno.stdout);
  ({ exit } = Deno);
}

if (runtime.startsWith("Node")) {
  readable = process.stdin;
  writable = new WritableStream({
    write(value) {
      process.stdout.write(value);
    },
  }, new CountQueuingStrategy({ highWaterMark: Infinity }));
  ({ exit } = process);
}

if (runtime.startsWith("Bun")) {
  readable = Bun.file("/dev/stdin").stream();
  writable = new WritableStream<Uint8Array>({
    async write(value) {
      await Bun.write(Bun.stdout, value);
    },
  }, new CountQueuingStrategy({ highWaterMark: Infinity }));
  ({ exit } = process);
}
node_modules/.bin/tsc --esModuleInterop index.ts
node_modules/@types/node/globals.d.ts:509:14 - error TS2300: Duplicate identifier 'fetch'.

509     function fetch(
                 ~~~~~

  node_modules/bun-types/globals.d.ts:1029:6
    1029  var fetch: Fetch;
              ~~~~~
    'fetch' was also declared here.

node_modules/@types/node/module.d.ts:360:13 - error TS2687: All declarations of 'dirname' must have identical modifiers.

360             dirname: string;
                ~~~~~~~

node_modules/@types/node/module.d.ts:366:13 - error TS2687: All declarations of 'filename' must have identical modifiers.

366             filename: string;
                ~~~~~~~~

node_modules/bun-types/bun.d.ts:117:8 - error TS2420: Class 'ShellError' incorrectly implements interface 'ShellOutput'.
  Property 'bytes' is missing in type 'ShellError' but required in type 'ShellOutput'.

117  class ShellError extends Error implements ShellOutput {
           ~~~~~~~~~~

  node_modules/bun-types/bun.d.ts:434:3
    434   bytes(): Uint8Array;
          ~~~~~~~~~~~~~~~~~~~~
    'bytes' is declared here.

node_modules/bun-types/globals.d.ts:1029:6 - error TS2300: Duplicate identifier 'fetch'.

1029  var fetch: Fetch;
          ~~~~~

  node_modules/typescript/lib/lib.dom.d.ts:28708:18
    28708 declare function fetch(input: RequestInfo | URL, init?: RequestInit): Promise<Response>;
                           ~~~~~
    'fetch' was also declared here.
  node_modules/@types/node/globals.d.ts:509:14
    509     function fetch(
                     ~~~~~
    and here.

node_modules/bun-types/globals.d.ts:1939:12 - error TS2687: All declarations of 'dirname' must have identical modifiers.

1939   readonly dirname: string;
                ~~~~~~~

node_modules/bun-types/globals.d.ts:1942:12 - error TS2687: All declarations of 'filename' must have identical modifiers.

1942   readonly filename: string;
                ~~~~~~~~

node_modules/bun-types/overrides.d.ts:3:29 - error TS2305: Module '"bun"' has no exported member 'PathLike'.

3 import type { BunFile, Env, PathLike } from "bun";
                              ~~~~~~~~

node_modules/typescript/lib/lib.dom.d.ts:16004:11 - error TS2430: Interface 'MessageEvent<T>' incorrectly extends interface 'Bun.MessageEvent<T>'.
  Types of property 'ports' are incompatible.
    Type 'readonly MessagePort[]' is not assignable to type 'readonly import("worker_threads").MessagePort[]'.
      Type 'MessagePort' is missing the following properties from type 'MessagePort': ref, unref, addListener, emit, and 13 more.

16004 interface MessageEvent<T = any> extends Event {
                ~~~~~~~~~~~~

node_modules/typescript/lib/lib.dom.d.ts:26068:11 - error TS2430: Interface 'WebSocket' incorrectly extends interface 'import("/home/xubuntu/bin/node_modules/@types/ws/index").WebSocket'.
  Types of property 'binaryType' are incompatible.
    Type 'BinaryType' is not assignable to type '"arraybuffer" | "nodebuffer" | "fragments"'.
      Type '"blob"' is not assignable to type '"arraybuffer" | "nodebuffer" | "fragments"'.

26068 interface WebSocket extends EventTarget {
                ~~~~~~~~~

node_modules/typescript/lib/lib.dom.d.ts:28708:18 - error TS2300: Duplicate identifier 'fetch'.

28708 declare function fetch(input: RequestInfo | URL, init?: RequestInit): Promise<Response>;
                       ~~~~~

  node_modules/bun-types/globals.d.ts:1029:6
    1029  var fetch: Fetch;
              ~~~~~
    'fetch' was also declared here.

Found 11 errors in 6 files.

Errors  Files
     1  node_modules/@types/node/globals.d.ts:509
     2  node_modules/@types/node/module.d.ts:360
     1  node_modules/bun-types/bun.d.ts:117
     3  node_modules/bun-types/globals.d.ts:1029
     1  node_modules/bun-types/overrides.d.ts:3
     3  node_modules/typescript/lib/lib.dom.d.ts:16004
panva commented 3 days ago

@guest271314 there are very valid reasons for module authors to want to run native node APIs in Node and Web APIs elsewhere. Hence the separation of node and "everything else". Just because you don't see the reason and need doesn't mean it is there.

I will not get into the marketing false claims about node compatibility here.

NB: experimental-default-type does nothing on this regard and if statements in the code increase bundle sizes with dead code for other than the intended runtime that can have its own bundle.

guest271314 commented 3 days ago

there are very valid reasons for module authors to want to run native node APIs in Node and Web APIs elsewhere. Hence the separation of node and "everything else". Just because you don't see the reason and need doesn't mean it is there.

Then just use node.

I will not get into the marketing false claims about node compatibility here.

Right. Bun !== Node.js !== Deno.

No matter how much those claims are said/shouted.

For one thing it's impossible to polyfill the internal node:crypto module.

NB: experimental-default-type does nothing on this regard and if statements in the code increase bundle sizes with dead code for other than the intended runtime that can have its own bundle.

Again, just use node. Why even try to support Bun?

panva commented 3 days ago

Why are you advocating for willing module authors not supporting bun when they can?

We're in this mess because of broken compat claims.

I am able to publish modules that work in web api runtimes as well as node (and older node without the web apis) all the same. All i ask for is that i don't have to enumerate every non-node runtime (a list of which is never final) in the package json and instead have the ability to provide a node-specific build and one target for non-node runtimes.

Today I can only do that if i keep on extending the package exports with every month's favourite new runtime flavour. 👎

guest271314 commented 3 days ago

@panva For one thing if you are using Node.js you are starting out at a deficit because Node.js default module loader is CommonJS. There's no CommonJS or require() in ECMA-262. So you have to begin by making exceptions for node.

Deno has WICG Import Maps, can fetch file: system protocol. Node.js doesn't have those features, without modifying Undici, and/or using Node.js-specific loaders. Bun's fetch doesn't support full-duplex streaming in WHATWG Fetch, Deno and Node.js.

ECMA-262 doesn't write out standard streams. That means however you look at it you are going to have to implement reading stdin and writing to stdout differently for every JavaScript engine/runtime/interpreter, if standard streams are implemented at all; e.g., neither Facebook's hermes or SerenityOS's LibJS js implement a way to read standard input. WinterCG is not dealing with that technical fact.

And so forth. Why I use node, deno, bun, qjs, tjs at the same time.

I shared my solution for writing runtime agnostic code.

Either use basic if conditions, or write N different scripts.

panva commented 3 days ago

This is not about the module CJS/ESM type flavour.

guest271314 commented 3 days ago

This is not about the module CJS/ESM type flavour.

Then kindly explain precisely what you are talking about.

To me package.json is a Node.js artifact that is not necessary at all.

What do you mean by "webapi"?

guest271314 commented 3 days ago

@panva I think the general solution is for Bun - and Node.js to implement WICG Import Maps.

panva commented 3 days ago

Then kindly explain precisely what you are talking about.

I did.

https://github.com/oven-sh/bun/issues/15053#issuecomment-2465004901

If you write your modules a different way and can't accept that others might have different ways of thinking of how interoperable modules should be structured, that's fine too

guest271314 commented 3 days ago

You are assuming that developers are using a package.json file at all. I don't, in general.

You can if you want to.

In Deno we don't need a package.json file at all. We can import and export from anywhere, locally and using the network, directly. Node.js doesn't like you to use network imports at all. So this has to work both ways. I don't see Node.js trying to be compatible with Bun or Deno.

guest271314 commented 3 days ago

@panva As an author of JavaScript runtime agnostic code my vision is the capability to import Deno into Node.js, Node.js into Bun, and so forth. So if you want to use Node.js-specific API's in Bun environment you should be able to import NodeJS as either the Microsoft TypeScript types, or directly in JavaScript.

The reality is JavaScript runtime maintainers do whatever they want to, without necessarily consulting other JavaScript runtime maintainers, with an eye to compatibility or interoperability.

I suspect that few in the WinterCG world are actually running bun, tjs, node, llrt, deno, workerd, et al. at the same time.

Node.js maintainers, from what I see, want the world to bend to legacy Node.js API's that might not actually do anything. E.g, check this https://github.com/oven-sh/bun/issues/14968 out re process.stdout.read(), which is effectively useless - even in Node.js world.

guest271314 commented 3 days ago

@panva

https://github.com/wintercg/runtime-keys/issues/18

it highlights its importance and issues in the module ecosystem stemming from runtimes such as Bun and Deno pretending to be Node.js

"pretending" to be Node.js?

Just implement WICG Import Maps in Node.js world. Then you do whatever you want. Done. Take Deno's lead in this area.

Outside of the browser

{
    "imports": {
      "test/file": "./x.json",
      "test/sub/file": "./z.json"
   }
 }
<script type="importmap">
  {
    "imports": {
      "test/file": "./x.json",
      "test/sub/file": "./z.json"
   }
 }
</script>
<script type="module">
  import x from "test/file"
  with {
    type: "json"
  };
  import z from "test/sub/file"
  with {
    type: "json"
  };
  console.log(new TextDecoder().decode(new Uint8Array([x, z])), import.meta.resolve("test/file"), import.meta.resolve("test/sub/file"), import.meta);
</script>

Bun might follow suit.

Not an issue in Deno.

This is not about the module CJS/ESM type flavour.

Sounds like this is a module loader issue.

Here's how to do whatever you want in Deno, Node.js, and Bun re specifers Intercepting and handling arbitrary static and dynamic Ecmascript import specifiers, protocols and file extensions.

With the caveat being Deno re dynamic import(), which I thought for a while was a non-conformant Deno implementation of ECMA-262 import(). However, because I kept asking about that I finally got an answer that pointed out that big ole or in ECMA-262 Is Deno's implementation of dynamic import() ECMA-262 and test262 conformant? #629

The host environment must perform FinishLoadingImportedModule(referrer, specifier, payload, result), where result is either a normal completion containing the loaded Module Record or a throw completion, either synchronously or asynchronously.

(Emphasis added)

Jarred-Sumner commented 3 days ago

@panva i'm sorry we haven't fixed the node:crypto bugs blocking the "node" target in Jose from working in Bun properly yet. We aren't going to change the decision to support the "node" condition, as in most cases it is better than the alternatives

If you want to force Bun to use the web api version, you can put an additional "bun" condition before the "node" condition and Bun will respect it

We will work on node:crypto more in about a month I expect

guest271314 commented 3 days ago

@Jarred-Sumner

Why is Bun trying to implement node:crypto? Isn't that basically impossible to implement outside of Node.js? [AskJS] Why is the internal crypto module so difficult to bundle as a standalone portable script?

The crypto module is not entirely Javascript. Some of its implementation uses native code which only runs in the nodejs environment and some of its implementation uses nodejs internals (like the thread pool).

A purely Javascript implementation would likely be much slower for some crypto operations.

We have webcrypto of node:crypto that is the standardized Web Cryptography API

import { webcrypto } from "node:crypto";

is supported by Node.js, Bun, and Deno, right now.

panva commented 3 days ago

@Jarred-Sumner thank you for thoughts. FWIW the issue here isn't necessarily about the jose module hardship, but rather about improving the situation for module authors.

Would you support the proposal from https://github.com/wintercg/runtime-keys/issues/18?

I believe the proposal [wintercg/runtime-keys] needs https://github.com/wintercg/runtime-keys/issues/5 combined with at least some text around how the keys are to be used.

For example:

add "wintercg" or similar which effectively behaves as "default" does in the Node.js resolution algorithm prescribe that runtimes must first look for their own key, then wintercg/default, and only then fallback on proprietary resolution algorithms

This would not encoach on the runtime's ability to still target "node" as the proprietary resolution algorithm path.