pcattori / vite-env-only

MIT License
116 stars 3 forks source link

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

Open gustavopch opened 8 months ago

gustavopch commented 8 months ago

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"),
   )
 }
bradymwilliams commented 6 months ago
diff --git a/node_modules/vite-env-only/dist/index.js b/node_modules/vite-env-only/dist/index.js
index 440090b..2fdec0f 100644
--- a/node_modules/vite-env-only/dist/index.js
+++ b/node_modules/vite-env-only/dist/index.js
@@ -280,6 +280,16 @@ var transform = (code, id, options) => {

 // src/macro.ts
 var maybe = (_) => {
+    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(
     [
       `${name}: unreplaced macro`,

thanks, added a patch for 2.2.0 using your fix

pcattori commented 4 months ago

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.

juliuxu commented 3 weeks ago

@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 commented 2 weeks ago

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 commented 2 weeks ago

@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: https://github.com/remix-run/remix/discussions/9790#discussioncomment-10625633

jrestall commented 2 weeks ago

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.