Use module export condition for types and textdocument

[remcohaszing]

The module export condition is an unofficial export condition that’s respected by bundlers. It tells the bundler to resolve to an entrypoint, regardless of whether a module is imported or required. This avoids the dual package hazard.

This was the original intent of the browser condition when it was added in #1326.

Thanks for telling me this @Andarist

[dbaeumer]

@aeschli can you please have a look.

[remcohaszing]

Any updates on this? If this is accepted, I would like to apply this to more VSCode related packages.

[dbaeumer]

@aeschli could you please have a look since you know more about this subject than I do :-)

[aeschli]

@remcohaszing with browser we want to provide the bits that should be used when bundling a language server (or client) that can run in the browser. These bits do not use any NodeJS APIs but browser APIs instead.
For types and text document these bits are the same. None of these modules has dependencies on NodeJS or browser API.
But client, server, jsonrpc do. We eventually also want to move to the exports syntax.

Shouldn't we keep the browser entry point then?

[Andarist]

The browser condition is fine - it's great that you include it. You describe its purpose well but the configuration of this package exports suggests something different. And like @remcohaszing has mentioned, the PR that introduced this browser condition states a different goal

Let's take a look at the defined exports:

	"exports": {
		".": {
			"browser": "./lib/esm/main.js",
			"import": "./lib/esm/main.js",
			"default": "./lib/umd/main.js"
		}
	},

Now all requests with the browser target resolve to ./lib/esm/main.js. But it's the same file to which an import in node can resolve. So I assume that this file is just quite independent from the environment.

But if we'd bundle with the node target we might still get both ./lib/esm/main.js (or we import this package somewhere) and ./lib/umd/main.js (if we require it at the same time somewhere else). A situation like this is not that uncommon in complex module graphs. Bundling for a node target is also a thing - it's a popular strategy when building VS Code extensions.

So this PR tries to avoid the above by replacing the browser condition with module condition. That condition would allow bundlers to always resolve to ./lib/esm/main.js regardless of the target (and regardless of how it was imported/required).

The browser condition can further be used with this (conditions "compose", you can nest them indefinitely) - but that kind of a thing has a different goal (the one that you have mentioned: you can load different code depending on the condition without affecting the public API etc). So all in all - those just solve two different problems.

[aeschli]

I'd like to keep a browser property. Even if not (yet) needed for types and document it will signal that we are ready to run in a browser environment.
Can you adjust the PR?

[remcohaszing]

With the addition of the module condition, the browser condition has no added value here. It merely blocks users from using something along the lines of node --require ./setup-jsdom.js --conditions browser --test.

Still, just adding the module conditions is an improvement on its own. I removed the browser conditions.

[Andarist]

It merely blocks users from using something along the lines of node --require ./setup-jsdom.js --conditions browser --test.

Conditions are like a priority list of sorts. I'd just reorder those conditions to address this problem. browser is more specific than module (it's a simplification since they just don't overlap perfectly, it's not like one is a strict subset/superset of another) so it should come first.

You could always have like browser: { module, import, default }. It just doesn't quite make sense since the browser condition isn't expected to kick in outside of the bundler scenario (some imports map resolver could also resolve to that, I guess). Since browsers don't understand CJS anyway it's somewhat OK to simplify this "path" to just the ESM build.

I see your argument with the JSDOM but I'm just not sure if that's a popular enough scenario to support it.

[dbaeumer]

@aeschli can you approve this if you think it is OK.