kentcdodds / babel-plugin-macros

🎣 Allows you to build simple compile-time libraries
https://npm.im/babel-plugin-macros
MIT License
2.62k stars 135 forks source link

Can we run babel-plugin-macros in the browser #186

Closed lukesmurray closed 2 years ago

lukesmurray commented 2 years ago

I am trying to transpile code containing a babel macro in a browser environment. The use case is for a live coding playground. Unfortunately when I try to use babelPluginMacros in a browser environment I run into issues because babelPluginMacros assumes a file system is available.

https://github.com/kentcdodds/babel-plugin-macros/blob/8fc6383ef04f628d0b799a9b281c5fc0693b25ac/src/index.js#L65-L73

Relevant code or config

import * as Babel from "@babel/standalone";
import babelPluginEmotion from "@emotion/babel-plugin";
import babelPluginMacros from "babel-plugin-macros";

// code to transpile containing a macro
const code = `import tw from 'twin.macro'; 
        render(<div css={tw\`bg-red-500\`}>I should have a red background!</div>)
`

Babel.transform(code, {
  presets: [
    [
      "react",
      { runtime: "automatic", importSource: "@emotion/react" },
    ],
  ],
  plugins: [
    [babelPluginEmotion, { sourceMap: false }],
    // the rest of this example works. I only get errors when I enable babelPluginMacros here
    babelPluginMacros,
  ],
})

What you did

tldr: monkey patching through webpack config but it isn't working

First I ran into issues with fs since I'm running in the browser, so I set my webpack config to not fallback for fs.

config.resolve.fallback.fs = false;

Then I ran into issues with fs.statSync which is called by resolve.sync that I linked to earlier. So I added an empty function mock for resolve.

 config.resolve.alias = "fbjs/lib/emptyFunction"

Now I'm getting an error for resolve.sync is not a function which makes sense cause I set resolve to be an empty function.

I can keep going down this monkey-patching rabbit hole but I feel like I must be doing something wrong.

Ideas for Solutions

  1. I see that macrosPlugin takes args. Is there any way for me to specify those? If I could override nodeResolvePath and require then I would be done.

https://github.com/kentcdodds/babel-plugin-macros/blob/8fc6383ef04f628d0b799a9b281c5fc0693b25ac/src/index.js#L80-L85

  1. If 1 doesn't work is there a way for me to mock or skip resolvePath. Once I get to interopRequire I think I can just alias it with webpack.

https://github.com/kentcdodds/babel-plugin-macros/blob/8fc6383ef04f628d0b799a9b281c5fc0693b25ac/src/index.js#L213-L215

I'm completely new to this stuff so would love any feedback from someone smarter than me.

Apologies for the issue on a dormant repo. If there is a better place to ask please let me know and I'll close this and move it.

lukesmurray commented 2 years ago

rubber ducked myself 🐣.

I figured out how to pass args.

              [
                babelPluginMacros,
                {
                  resolvePath: () => {
                    console.log("resolving path");
                  },
                },
              ],

I'll reopen if I have more questions but this might be resolvable on my own.

conartist6 commented 2 years ago

Thanks for sharing. Perhaps your experience will help someone else out down the line!

lukesmurray commented 2 years ago

Thanks @conartist6. I'm actually very close to running babel-plugin-macros in the browser with limited hacks. However I ran into one issue that seems quite possible to fix.

babel-plugin-macros calls getConfigFromFile(configName, filename) to resolve the config. However in a browser environment there is no file system so this function throws an error. I'm not sure if you would ever want this function called in the browser since if you're running babel manually you can just pass the config as options.

https://github.com/kentcdodds/babel-plugin-macros/blob/8fc6383ef04f628d0b799a9b281c5fc0693b25ac/src/index.js#L300-L302

Would you be open to discussing changes to prevent getConfigFromFile from being called in a browser environment? For example we could check if window is defined prior to calling the function. An alternative approach would be to allow users to pass their own version of getConfigFromFile similar to what you already do with require and resolvePath. Open to other ideas, and I know this is fairly niche but if the changes are small enough it seems like it opens interesting possibilities for live coding playgrounds that rely on babel-macros.

This is the only issue preventing me from running babel-plugin-macros in the browser right now which is pretty incredible so cheers 🍻.

conartist6 commented 2 years ago

How is it failing right now? It looks to me like the code should be more or less fine when the file is not found. Also I notice that you've pointed to a spot where configName is true, which means a macro that you're using has declared that it either offers or requires configuration. Either way you can still provide it through the getConfigFromOptions code path, which is designed to work in place of a config file (I needed it to when I wrote it).

lukesmurray commented 2 years ago

We're on the same page about passing the options. The error isn't due to failure to find the file. The error is due to getConfigFromFile calling cosmiconfig which is not designed to run in the browser.

Here is the error I get.

./node_modules/resolve-from/index.js:3:0
Module not found: Can't resolve 'module'

Import trace for requested module:
./node_modules/import-fresh/index.js
./node_modules/cosmiconfig/dist/loaders.js
./node_modules/cosmiconfig/dist/index.js
./node_modules/babel-plugin-macros/dist/index.js

Cosmiconfig requires import fresh which calls resolveFrom which requires module.

Unfortunately module is a node built-in so it fails to resolve in the browser.

I can try to get around this by mocking module, but it gets insanely messy. Would be way easier if we could somehow skip the cosmiconfig search altogether when we're in a browser environment.

It does look like the current code works even when the config isn't found which makes two solutions possible.

  1. check if we're in the browser and if we are then return an empty object from getConfigFromFile rather than running cosmiconfig.
  2. allow the user to pass their own implementation of getConfigFromFile with a fallback to the current getConfigFromFile similar to what is already in place with require and resolve.

Hopefully that makes sense?

conartist6 commented 2 years ago

Everything about cosmiconfig, including its require and attempt to find a config file, is wrapped in a try catch block: https://github.com/kentcdodds/babel-plugin-macros/blob/8fc6383ef04f628d0b799a9b281c5fc0693b25ac/src/index.js#L269-L280

It only throws the error because it cannot find any config: https://github.com/kentcdodds/babel-plugin-macros/blob/8fc6383ef04f628d0b799a9b281c5fc0693b25ac/src/index.js#L304-L316

You could suppress the error for now just by providing an empty config.

lukesmurray commented 2 years ago

TLDR: got it working but agree the error looks like it should be caught so closing til I can figure out whats going on.

Everything about cosmiconfig, including its require and attempt to find a config file, is wrapped in a try catch block.

It definitely looks like the error I showed should be getting caught there and I don't have a good explanation for why it isn't.

You could suppress the error for now just by providing an empty config.

Unfortunately I get the module resolution error inside that try catch statement 🤔 so I don't even hit the empty config issue.

My best guess is this has something to do with the fact that I'm using nextjs. Maybe they have an early exit or something if any module fails to resolve during server side rendering so we're not even hitting the try catch statement. I'm not sure 🤷‍♂️ .

I wrote a patch which implements option 1 and checks if we're in a browser environment before attempting to find a config file. The patch just wraps the try-catch statement you linked with if (typeof window === 'undefined').

diff --git a/node_modules/babel-plugin-macros/dist/index.js b/node_modules/babel-plugin-macros/dist/index.js
index 4501c0e..bf2fb0c 100644
--- a/node_modules/babel-plugin-macros/dist/index.js
+++ b/node_modules/babel-plugin-macros/dist/index.js
@@ -252,19 +252,21 @@ function applyMacros({
 }

 function getConfigFromFile(configName, filename) {
-  try {
-    const loaded = getConfigExplorer().search(filename);
-
-    if (loaded) {
+  if (typeof window === 'undefined') {
+    try {
+      const loaded = getConfigExplorer().search(filename);
+
+      if (loaded) {
+        return {
+          options: loaded.config[configName],
+          path: loaded.filepath
+        };
+      }
+    } catch (e) {
       return {
-        options: loaded.config[configName],
-        path: loaded.filepath
+        error: e
       };
     }
-  } catch (e) {
-    return {
-      error: e
-    };
   }

   return {};

With that change I was able to get a babel macro running in the browser.

twin-in-the-browser

Not sure where to go from here. Its quite possible that this is an issue on my end so I'll close for now until I can get more info.

lukesmurray commented 2 years ago

Thanks for spending the time to look at this. I know it is an absolute wall of text 😅

conartist6 commented 2 years ago

Certainly! I spend a lot of time building open source things (mostly not this) but I really like getting a glimpse into the ways people are using things and whether the designs I support hold up to unforeseen conditions and challenges. In this case I think causally related errors would have helped a lot in figuring out what's going on. It's difficult to understand the flow of exception states when throw doesn't include a trace, which is what happens when you catch an error and rethrow the exact same error (the second throw is not traced).

lukesmurray commented 2 years ago

Not sure if I'll have the bandwidth to follow up on this but to close this out I published a demo with babel-plugin-macros running in the browser https://twin-playground.vercel.app/ and I open sourced the code in this repo.

All of the weird hacks I did to get this working are explained in code comments on this commit.