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

Don't throw unreplaced macro when running without Vite #19

Open
gustavopch opened this issue Jan 24, 2024 · 9 comments · May be fixed by #42
Open

Don't throw unreplaced macro when running without Vite #19

gustavopch opened this issue Jan 24, 2024 · 9 comments · May be fixed by #42

Comments

@gustavopch
Copy link

gustavopch commented Jan 24, 2024

I am using serverOnly$ to ensure Vite doesn't bundle a function that's supposed to be executed on the server. But I actually also need to run that function in a simple Node.js script that doesn't use Vite. When I run my script, I get this error:

Error: vite-env-only: unreplaced macro

Did you forget to add the 'vite-env-only' plugin to your Vite config?

So I propose that that error is only thrown when running via Vite.

Not sure if there's a better way to check, but possibly something like this:

 const maybe = <T>(_: T): T | undefined => {
+  const usingVite = import.meta.env &&
+    'MODE' in import.meta.env &&
+    'BASE_URL' in import.meta.env &&
+    'PROD' in import.meta.env &&
+    'DEV' in import.meta.env &&
+    'SSR' in import.meta.env
+
+  if (!usingVite) {
+    return _;
+  }
+
   throw Error(
     [
       `${pkgName}: unreplaced macro`,
       "",
       `Did you forget to add the '${pkgName}' plugin to your Vite config?`,
       "👉 https://github.com/pcattori/vite-env-only?tab=readme-ov-file#installation",
     ].join("\n"),
   )
 }
@pcattori
Copy link
Owner

Originally, I hesitated to implement this feature since it felt like the error provided a base layer of safety.
But when I tried to define what I meant by "safety" I realized that the error doesn't actually prevent the code from being included in the module graph.
It only replaces the runtime error, but not the code shipped to that environment.

An alternative I thought of was to have unsafe_serverOnly$(...) or serverOnly$(..., { unsafe: true }) or something like that.
But that means opting in or out of safety where the macro is used, which is the wrong place; the whole point is that the same macro gets accessed sometimes from within Vite and sometimes outside of Vite.

Another option would be to have a VITE_ENV_ONLY=server or similar env var so that the error is only thrown when the macro is used outside of the expected env. But again, this doesn't prevent code from being included in the module graph.

So to summarize, I think you're right that a simple runtime check for Vite is the best approach. If you are using the Vite plugin, things continue to work the same way. If you aren't using the Vite plugin, there's no bundling going on so there's no module graph artifact to try to protect in the first place.

@pcattori pcattori linked a pull request May 24, 2024 that will close this issue
@juliuxu
Copy link

juliuxu commented Sep 10, 2024

@pcattori Hey, any chance the original changes in the PR you made can be merged? I see you proposed a smarter solution, but perhaps this check is good enough to merge?

My team has the same problem, we are including some share code in our unit tests which triggers this error. We have been mitigating this by using patch-package as proposed. It works fine enough, but I'd really want to get it removed and have the fix/solution upstream instead.

@jrestall
Copy link

We are also hitting this issue, we have some code shared between our expressjs remix server and the remix bundle itself. The expressjs code is compiled with esbuild and therefore throws when using the same code, since there is no Vite plugin to remove the $serverOnly macros.

A solution that ensures $serverOnly doesn't throw when built outside of Vite such as with esbuild would be great.

@gustavopch
Copy link
Author

@jrestall Besides the patch I provided in the first message, you can also work around by moving as much of your Express server code to the bundle as possible as described here: remix-run/remix#9790 (reply in thread)

@jrestall
Copy link

Really appreciate the tip @gustavopch, thank you! Moving all our expressjs server code to entry.server would be a very large change across our apps but it's good to know there's another option. We'll go with the patch for now and hope the behavior is fixed.

@zwhitchcox
Copy link

Is there a temporary workaround? Or do we have to build from source with the patch? I don't see why we would care about people not using vite. The plugin is called vite-env-only.

@gustavopch
Copy link
Author

@zwhitchcox You don't need to build from source to patch. You can use https://npm.im/package/patch-package or something similar.

@zwhitchcox
Copy link

@gustavopch Thanks!

@SylRob
Copy link

SylRob commented Dec 26, 2024

hitting this issue with playwright running e2e with dev server.
The patch indeed fix the issue but it would be great to have an official release for that

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

Successfully merging a pull request may close this issue.

6 participants