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

Configure module resolution condition in developement mode #1930

Open
garronej opened this issue Oct 21, 2024 · 5 comments
Open

Configure module resolution condition in developement mode #1930

garronej opened this issue Oct 21, 2024 · 5 comments
Labels

Comments

@garronej
Copy link

garronej commented Oct 21, 2024

Description

Hello Vike team,

First of all, congratulations on what you’re building!

I wanted to bring to your attention a potential issue with the Vike development server configuration, which could lead to a "dual package" scenario during development. By "dual package," I mean instances where different distributions of the same module are loaded simultaneously. This can cause problems, particularly for modules that are used as peer dependencies, such as those exposing a React context provider.

To help illustrate the issue, I’ve created a reproduction repository:
https://github.com/garronej/vike-dual-package-repo-repo

As per @Andarist's analysis, this issue could potentially be resolved on your side by passing the --conditions=development flag to Node.js. You can review his explanation here.

This issue is currently affecting users implementing tss-react in Vike, and likely other modules that rely on @emotion/react as a peer dependency.

Thanks a lot for taking the time to review this!

Best,

References:

@Andarist
Copy link

I believe that all meta-frameworks should configure the conditions like this. Some already do. So by doing that the tool can make sure that it behaves like the rest of the ecosystem, thus making it easier for the users. It's especially true in this situation because a part of the overall app already has those conditions enabled, so not enabling them for the other part is going to be confusing for people

@brillout
Copy link
Member

Thanks for reaching out and that makes a lot of sense 👍

I believe that all meta-frameworks should configure the conditions like this. Some already do.

Do you know how they do it? Because I ain't aware of a simple way.

@Andarist
Copy link

I've done short research and now I'm not sure how many metaframeworks do this already. Many are but to a great extent they might rely on bundling the server code. In such a scenario, the resolution algorithm is offloaded to a bundler and both app-level code and node_modules code is loaded using the same conditions.

If you want to keep loading some modules using node itself then I think you might have 2 options:

  • use a wrapper script that proxies to your real thing but adds --conditions development/--conditions production when appropriate
  • use loader hooks (docs):

The conditions property in context is an array of conditions for package exports conditions that apply to this resolution request. They can be used for looking up conditional mappings elsewhere or to modify the list when calling the default resolution logic.

Note that the problem itself is not isolated to development/production conditions. It depends on what kind of direct control you are giving to users here but when targeting different environments (such as even cloudflare workers or deno) you should also apply respective conditions. This is what, for example, Remix is doing (see their serverConditions but also note that they preconfigure that on behalf of the user based on other settings).

@brillout brillout added blocked-by-vike-cli enhancement ✨ New feature or request and removed bug 💥 labels Oct 27, 2024
@brillout
Copy link
Member

It seems that we can make it work, except of one blocking issue: the user would need to set all resolve conditions when he calls node dist/server/index.js --conditions .... (There is also another blocker internal to Vike but it's going to be resolved soon.)

Alternatively, spawning a child process in production would do the trick but I wonder whether it could break certain production Node.js environments? For example docker?

  • use loader hooks (docs):

Do you have an example of how to use register() for adding resolve conditions? That would be a neat solution.

Also, there is the environment variable NODE_OPTIONS to set Node.js options but I guess modifying this environment variable after the process has started is too late, so I guess it isn't a solution.

Remix is doing (see their serverConditions

Remix only passes serverConditions to esbuild and doesn't pass it to Vite nor Node.js. So I still wonder, is there any framework out there that sets the development/production resolve conditions?

@garronej
Copy link
Author

Hello @brillout,

So I still wonder, is there any framework out there that sets the development/production resolve conditions?

At least Next does.

Here is what we get with the same test case in Next App Router:

image

image

Here is the repo if you want to test: https://github.com/garronej/next-dual-package-repro-repo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants
@brillout @garronej @Andarist and others