feat: support a `vite` entry point (export condition / main field) #15643

milesj · 2024-01-18T23:56:35Z

Description

After speaking with @yyx990803, we're looking for a way to better support local packages (via monorepo/workspaces) without a build step. The current suggestion is to set resolve.aliases, but what if we leaned on the package ecosystem more?

This PR adds support for a vite condition for package exports, and a vite main field (when not using exports). Both patterns can point to a source file, instead of a built file. These vite entry points will only be resolved for local packages, and not packages found in node_modules.

Furthermore, for backwards compatibility, I wrapped this in a vitePackageEntryPoints experiment for now.

For more information, look at the docs in this PR.

Additional context

What is the purpose of this pull request?

Bug fix
New Feature
Documentation update
Other

Before submitting the PR, please make sure you do the following

Read the Contributing Guidelines, especially the Pull Request Guidelines.
Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
Update the corresponding documentation if needed.
Ideally, include relevant tests that fail without this PR but pass with it.

stackblitz · 2024-01-18T23:56:38Z

Run & review this pull request in StackBlitz Codeflow.

milesj · 2024-01-19T00:01:34Z

playground/monorepo-alias/__tests__/alias.spec.ts

@@ -0,0 +1,10 @@
+import { expect, test } from 'vitest'


This playground tests normal resolve.alias usage.

milesj · 2024-01-19T00:02:09Z

playground/monorepo-vite-condition/__tests__/resolve.spec.ts

@@ -0,0 +1,38 @@
+import { describe, expect, test } from 'vitest'


This playground tests main/exports vite usage. There's many different patterns.

bluwy · 2024-01-22T07:04:37Z

I'm not a fan of introducing Vite-specific metadata to the package.json. Even if it's local-only, the fields are also likely to be published and would be exposed and even point to a non-existing source file?

I think publishConfig could be used instead for this case: pnpm docs and npm docs. You can then split out your dev and publish-only fields this way.

dominikg · 2024-01-22T15:13:54Z

so the vite condition would only be used during development? wouldn't it be easier to use the existing development condition then? https://nodejs.org/api/packages.html#community-conditions-definitions

I agree with @bluwy that we should not add a new top-level field to package.json. This feature will only be used by new projects/adopted, so if it is only supported via exports maps that should be fine. Resolving via top level package.json fields is basically a legacy option today, no need to add new fields to it.

milesj · 2024-01-22T18:09:11Z

@bluwy Interesting suggestion, but I don't see how publishConfig would work for something that is local/dev only. I'd also argue that export conditions are the preferred way to support tool-specific logic. It's exactly why custom conditions are allowed.

@dominikg The development condition kind of assumes a JS file that can be evaluated at runtime. Throwing in TS files, or .vue files (maybe), or something else, kind of breaks that guarantee. It also just exists for NODE_ENV checks.

I'm cool with removing the top-level vite but the exports condition is still viable. Maybe source or bundler for the export condition, instead of vite? That way it's generic, and can be used by other tools.

bluwy · 2024-01-23T07:31:34Z

@bluwy Interesting suggestion, but I don't see how publishConfig would work for something that is local/dev only. I'd also argue that export conditions are the preferred way to support tool-specific logic. It's exactly why custom conditions are allowed.

You can configure it like this:

{
  "name": "lib",
  "main": "./src/index.js",
  "publishConfig": {
    "main": "./dist/index.js"
  }
}

Conditions are indeed the more reliable way to declare them, but I don't think it's best for a tool that pushes towards standards. Encouraging the vite condition feels like the start of introducing proprietary APIs in libraries that we had always refrained from.

dominikg · 2024-01-23T08:17:34Z

Also not keen about adding on to the pile of custom conditions. The landscape there is messy already and instead of adding more ways we should focus on enabling projects to migrate towards clean esm and providing tooling on top of existing conditions to make it work.

case in point: Webpack made the "module" condition a thing https://webpack.js.org/guides/package-exports/#conditions and appearantly wmr also made "esmodules". ugh.

All this does is creating more fragmentation some sort of lock-in mechanism where either all bundlers are forced to support each others inventions or projects will be tied to the one they started on forever.

If you are using exports conditions, regardless of vite or an existing one, you would have to ship the referenced files in published packages too, as you have no control over the conditions used on the consumers side. So for public packages i think publishConfig is the way to go. For local only packages i think the development or production condition can work.

milesj · 2024-01-25T00:47:04Z

@yyx990803 @bluwy @dominikg

Took a step back to reimplement this. I agree, exports adds a lot of weird overhead. It's also a lot for consumers to configure, so I thought, why can't we just automate this?

And that's exactly what I did. With my previous attempt, we were able to determine which packages are local or not. So I expanded this to automatically resolve source files of local packages, with no configuration from the consumer! This actually worked really well in the playground.

bluwy

Thanks for tackling this from a different perspective. Besides my comments/concerns below, additionally I think from our last meeting, we should only need improved documentation on setting up monorepos in Vite.

I don't think there's something Vite can do internally or automatically to make it just work without injecting Vite-specific APIs or heuristics, which we want to avoid. So I think it's best we only do the documentation part, which is also tracked at #10447

bluwy · 2024-01-25T14:30:58Z

packages/vite/src/node/plugins/resolve.ts

+  // We need to support array conditions like `["./*.js", "./*.mjs"]`
+  // So loop through each result and find one that actually exists
+  if (result.length > 1) {
+    for (const entry of result) {
+      if (fs.existsSync(path.join(pkg.dir, entry))) {
+        return entry
+      }
+    }
+  }


We currently intentionally don't support this as it's not following the spec. Node.js only specifies to use the next element if the specifier is invalid, not if it don't exist on the fs.

Good to know, I can remove it.

bluwy · 2024-01-25T14:32:57Z

packages/vite/src/node/packages.ts

+  // We do this check here, so that it only runs once per package,
+  // and not for every file in the package!
+  if (inWorkspace) {
+    for (const srcName of ['src', 'sources']) {


I don't think Vite should be hardcoding these directories. The users should be free to arrange the output however they want.

I agree. I originally started adding this behind a config setting, similar to file extensions, but pushed it off to get the POC working.

milesj · 2024-01-25T18:00:00Z

Let's wait and see what Evan's thoughts on this are. I was tasked in finding a way to make monorepo setups "just work" without extra configuration (this includes alias mapping). I think for the majority of consumers, the automatic sources will work just fine. For others, they can just disable it, something like:

resolve: {
  localSources: false
}

We could even expand the config and give consumers more control. But I feel like this is getting away from the original goal.

resolve: {
  localSources: ['package-a', 'package-b'],
  srcDirs: ['lib', 'app'],
}

bluwy · 2024-05-09T04:04:05Z

Closing this as the Vite team had discussed with Evan and this isn't the way we want to go for now.

milesj changed the title ~~new: Support a vite entry point (export condition / main field)~~ feat: Support a vite entry point (export condition / main field) Jan 18, 2024

milesj commented Jan 19, 2024

View reviewed changes

milesj changed the title ~~feat: Support a vite entry point (export condition / main field)~~ feat: support a vite entry point (export condition / main field) Jan 19, 2024

patak-dev added the p3-significant High priority enhancement (priority) label Jan 24, 2024

milesj added 9 commits January 24, 2024 14:12

Update resolve.

ebabdf2

Add playground.

7434427

Add another playground.

e982306

Change workspace check.

ab01a0d

Fix resolve.

2f89104

Polish.

244e46f

Add more tests.

fa1ab73

Add doc page.

fecb677

Wrap in experiment.

5354cd4

milesj force-pushed the vite-export-condition branch from 7a71420 to 5354cd4 Compare January 24, 2024 22:25

milesj added 4 commits January 24, 2024 16:02

Add auto src searching.

5f69bf2

Remove old usage.

4b82710

Redo playground.

b18666f

Polish.

6b8fcb3

bluwy reviewed Jan 25, 2024

View reviewed changes

bluwy closed this May 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support a `vite` entry point (export condition / main field) #15643

feat: support a `vite` entry point (export condition / main field) #15643

milesj commented Jan 18, 2024 •

edited

stackblitz bot commented Jan 18, 2024

milesj Jan 19, 2024

milesj Jan 19, 2024

bluwy commented Jan 22, 2024

dominikg commented Jan 22, 2024 •

edited

milesj commented Jan 22, 2024 •

edited

bluwy commented Jan 23, 2024

dominikg commented Jan 23, 2024

milesj commented Jan 25, 2024 •

edited

bluwy left a comment

bluwy Jan 25, 2024 •

edited

milesj Jan 25, 2024

bluwy Jan 25, 2024

milesj Jan 25, 2024

milesj commented Jan 25, 2024

bluwy commented May 9, 2024

		@@ -0,0 +1,38 @@
		import { describe, expect, test } from 'vitest'

feat: support a vite entry point (export condition / main field) #15643

feat: support a vite entry point (export condition / main field) #15643

Conversation

milesj commented Jan 18, 2024 • edited

Description

Additional context

What is the purpose of this pull request?

Before submitting the PR, please make sure you do the following

stackblitz bot commented Jan 18, 2024

milesj Jan 19, 2024

Choose a reason for hiding this comment

milesj Jan 19, 2024

Choose a reason for hiding this comment

bluwy commented Jan 22, 2024

dominikg commented Jan 22, 2024 • edited

milesj commented Jan 22, 2024 • edited

bluwy commented Jan 23, 2024

dominikg commented Jan 23, 2024

milesj commented Jan 25, 2024 • edited

bluwy left a comment

Choose a reason for hiding this comment

bluwy Jan 25, 2024 • edited

Choose a reason for hiding this comment

milesj Jan 25, 2024

Choose a reason for hiding this comment

bluwy Jan 25, 2024

Choose a reason for hiding this comment

milesj Jan 25, 2024

Choose a reason for hiding this comment

milesj commented Jan 25, 2024

bluwy commented May 9, 2024

feat: support a `vite` entry point (export condition / main field) #15643

feat: support a `vite` entry point (export condition / main field) #15643

milesj commented Jan 18, 2024 •

edited

dominikg commented Jan 22, 2024 •

edited

milesj commented Jan 22, 2024 •

edited

milesj commented Jan 25, 2024 •

edited

bluwy Jan 25, 2024 •

edited