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

Cannot apply macros when imported from a module outside of default resolution paths #87

Closed
brandonc opened this issue Oct 25, 2018 · 37 comments

Comments

@brandonc
Copy link

  • babel-plugin-macros version: 2.4.2
  • node version: 8.9.4
  • npm (or yarn) version: npm 5.6.0

Hi, Kent! Macros are super cool. Thanks for building this plugin. lingui-js is a really cool use case that I've been integrating lately. It uses macros to identify and extract translatable strings at build time, which simplifies react internationalization a great deal. However, I've run into an issue regarding node resolve that I could use your advice on before I try and submit a patch.

Relevant code or config

I have a project that loads modules into my bundle using a webpack alias for a directory outside of my project root. These modules in turn import macros. For example: require("#EXTERNAL_PLUGINS/index.js")

where #EXTERNAL_PLUGINS is a webpack alias for something like "../external_plugins"

Babel fails to load these modules if they import a macro because of a recent change to applyMacros that adds resolve.sync with a basedir set as the source file directory (which again, is outside my working folder and therefore doesn't have node_modules)

Here is the change:

1785a0f#diff-1fdf421c05c1140f6d71444ea2b27638R155

What happened:

resolve.sync throws an exception because it is unable to locate the macro with the configured basedir. Everything works correctly when using babel-plugin-macros 2.4.1

Reproduction repository:

https://github.com/brandonc/load-macros-outside-package

Suggested solution:

I took a stab at a fix but was unable to find a suitable basedir to use with resolve.sync. This is a hard problem with zero configuration. Looking for ideas from you for a suitable change.

@brandonc brandonc changed the title Cannot apply macros when imported from a module outside of Cannot apply macros when imported from a module outside of default resolution paths Oct 25, 2018
@audiolion
Copy link

I am not sure if this is related, but I am trying to go from CRA 2.0-alpha to 2.0 and they dropped support for .graphql imports and monorepo support. As a result my monorepo is now trying to use babel-plugin-macros with graphql-tag.macro and I am finding that macros aren't compiling when imported from symlinked packages, this might just be CRA though

e.g.
import usersQuery from '@monorepo/common/queries/users';

where that file is

import gql from 'graphql-tag.macro';

export default gql`
  query users {
    id
  }
`;

@almilo
Copy link

almilo commented Nov 20, 2018

@kentcdodds I am trying to do exactly the same (allowing macros to be loaded from custom directories) and had a look at the plugin code to send a PR.
My solution would be to add a moduleDirectory option to the plugin and extend the resolve configuration here: https://github.com/kentcdodds/babel-plugin-macros/blob/master/src/index.js#L155 so that resolve takes those directories into consideration.
That would require loading the configuration eagerly for the plugin instead of lazily for the macros. Is that ok / makes sense? If so, I would be happy with sending a PR.

@kentcdodds
Copy link
Owner

That's an interesting idea @almilo! I think we may be able to solve @Andarist's issue as well.

I'm ok with eagerly resolving the config.

What if the config looked like:

// babel-plugin-macros.config.js
module.exports = {
  options: {
    moduleDirectories: ['node_modules', 'shared'],
    alias: {
      'pipeline.macro': require.resolve('./index.js'),
    }
  }
}

The moduleDirectories could just be passed as the moduleDirectory option to resolve (and would default to 'node_modules'), but the alias one may be a little more tricky, but not too difficult I think. Would you be interested in doing that?

@wintercounter
Copy link
Contributor

wintercounter commented Aug 7, 2019

I was strugling with the same issue.

My ugly solution until there's a better option (babel config is dynamically generated for us, added this to that script):

const macrosPluginPath = require.resolve('babel-plugin-macros')
fs.readFile(macrosPluginPath, 'utf8', function(err, data) {
    fs.writeFile(
        macrosPluginPath,
        data.replace(
            'return resolve.sync(source, {',
            `return resolve.sync(source, {
                 paths: [p.resolve(__dirname, '../../'), p.resolve(process.cwd(), 'node_modules')],
            `
        ),
        'utf8',
        () => {}
    )
})

It's a REALLY, REALLY BAD solution. Once support will be added to customize this please allow us to configure this at babelrc at plugins definition, not only through cosmiconfig files.

wintercounter pushed a commit to wintercounter/mhy that referenced this issue Aug 7, 2019
@topaxi
Copy link

topaxi commented Oct 30, 2019

I just ran into this issue, this makes it pretty hard to use macros within a project, unless using relative imports everywhere.

@Darmody
Copy link

Darmody commented Nov 24, 2019

How's this going ? ran into this issue too.

@wintercounter
Copy link
Contributor

wintercounter commented Nov 24, 2019 via email

@jsphstls
Copy link

jsphstls commented Jul 10, 2020

I might be seeing this same issue. I am happy to open a new issue otherwise. I am trying to use relay in a monorepo with yarn workspaces. My relay implementation, graphql schema, and the queries live in one package that is compiled by Typescript and then imported into a sibling package. The sibling is a react app that render the wrapped relay component and refers to the queries by a key.

In the relay wrapper package:

 "devDependencies": {
    "babel-plugin-relay": "^9.1.0",
    "react": "16.13.1",
    "react-dom": "16.13.1",
    "react-relay": "^9.1.0",
    "react-scripts": "3.4.0",
    "relay-compiler": "^9.1.0",
    "relay-compiler-language-typescript": "^12.0.3",
    "relay-config": "^9.1.0",
    "relay-runtime": "^9.1.0",
    "typescript": "^3.9.5",

react-scripts start works just fine in the relay wrapper.

In the sibling, I see this terminal output:

Compiled with warnings.

/node_modules/babel-plugin-macros/dist/index.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

/node_modules/import-fresh/index.js
Critical dependency: the request of a dependency is an expression

In the browser:

MacroError: The macro you imported from "undefined" is being executed outside the context of compilation with babel-plugin-macros. This indicates that you don't have the babel plugin "babel-plugin-macros" configured correctly. Please see the documentation for how to configure babel-plugin-macros properly: https://github.com/kentcdodds/babel-plugin-macros/blob/master/other/docs/user.md

  1 | import graphql from 'babel-plugin-relay/macro';
  2 | export const someQuery = graphql `
> 3 |   query someQuery {

Since the macro works in its local package, but not when imported into another, I suspect it is related to this issue.

@wintercounter
Copy link
Contributor

wintercounter commented Jul 10, 2020 via email

@jsphstls
Copy link

@wintercounter that's a good thought. I see that babel-plugin-relay is still targeting ^2.0.0 of this package. Despite that, I seem to be resolving to 2.8.0 anyways.

yarn why v1.22.4
=> Found "babel-plugin-macros@2.8.0"
   - "_project_#babel-plugin-relay" depends on it
   - Hoisted from "_project_#babel-plugin-relay#babel-plugin-macros"
   - Hoisted from "_project_#react-scripts#babel-preset-react-app#babel-plugin-macros"
   - Hoisted from "_project_#@storybook#react#@storybook#core#babel-plugin-macros"
   - Hoisted from "_project_#@storybook#react#@storybook#core#babel-plugin-emotion#babel-plugin-macros"

@watery
Copy link

watery commented Jan 21, 2021

I'm getting this for a macro that's inside a dependecy - at least I think that's the source of the problem.
That dependecy has preval-macro as a dependency and not as a devDependency (because the latter caused a "[...] cannot find preval-macro [...]" compilation error).

Using it inside a create-react-app webapp.

Some version info (can't really tell which one matters):

  • <project>\node_modules\babel-plugin-preval\node_modules\babel-plugin-macros @ 2.8.0
  • <project>\node_modules\babel-plugin-preval @ 5.0.0
  • <project>\node_modules\babel-plugin-macros @ 2.5.0
  • <project>\node_modules\react-scripts @ 2.1.8

Moving to react-scripts @ 4.0.1 those have been updated to:

  • <project>\node_modules\babel-plugin-preval\node_modules\babel-plugin-macros disappeared
  • <project>\node_modules\babel-plugin-preval @ 5.0.0
  • <project>\node_modules\babel-plugin-macros @ "2.8.0

But I get the same error:

MacroError: The macro you imported from "undefined" is being executed outside the context of compilation with babel-plugin-macros. This indicates that you don't have the babel plugin "babel-plugin-macros" configured correctly. Please see the documentation for how to configure babel-plugin-macros properly: https://github.com/kentcdodds/babel-plugin-macros/blob/master/other/docs/user.md

That's a runtime error, that shows only when the page with the component is opened in the browser.

I'm looking for guidance about what to do / inspect.

@conartist6
Copy link
Collaborator

Well I don't necessarily know anything you don't, but I see that earlier in this thread a PR was submitted. There's some useful discussion on the PR about what was causing the issue, and also the PR shows that a fix was released in 2.8.0. If it were me I'd open up node_modules/babel-plugin-macros/dist/index.js and toss a debugger on the lines that are throwing the error. It looks to me like they are these lines. If you can report back as to how those lines are failing in your environment then we can help you determine the appropriate fix.

@watery
Copy link

watery commented Jan 21, 2021

Yep, I'm trying to do that but I'm failing since hours. I have both the Node inspector activated and the client (browser) DevTools open with breakpoints, but none are being triggered. I must be missing something obvious.

@conartist6
Copy link
Collaborator

Can you share exactly what you're running?

@conartist6
Copy link
Collaborator

If you're trying to debug this through the start script, maybe try the build script? Is it reproducible there?

@watery
Copy link

watery commented Jan 21, 2021

The uber-simplified case is this:

I have MyCompo

// ...
import preval from 'preval.macro';
// ...

class MyCompo extends React.Component {

    constructor(props) {
        super(props)

        this.state = {
            buildTime: preval`module.exports = new Date()`,
        }
    }

   // ...

Since MyCompo uses React (or at least I didn't find how to else do it), it is Babel compiled (transpiled) and published to a private (local) Verdaccio-based private npm repository.

Then the component in installed into MyApp

  "dependencies": {
    "mycompo": "^0.1.3",

and it's imported in the final application;

import MyCompo  from 'mycompo';
// ...
<MyCompo  />

Yes, I'm using the start script and I finally found that it is not passing inside the nodeResolvePath() function mentioned in the PR (the client flow at least).

I succeeded at adding a breakpoint at the line if (!macro.isBabelMacro) { you pointed out, and there the isBabelMacro property is undefined.

@conartist6
Copy link
Collaborator

I just meant for debugging, but I guess you figured it out. It doesn't really matter what's in your files I think, just where they're located. And you knew you'd see that undefined value because you see the error. The question is why is it undefined?

@watery
Copy link

watery commented Jan 21, 2021

The question is why is it undefined?

I'm actually stuck there, I can't understand the call stack and determine who's setting (or not setting) it.

@conartist6
Copy link
Collaborator

Well you said macro.isBabelMacro is undefined. What is macro...???

@watery
Copy link

watery commented Jan 21, 2021

Well you said macro.isBabelMacro is undefined. What is macro...???

Ah, I actually pasted the wrong line of code!

I meant isBabelMacrosCall in if (!isBabelMacrosCall) { -were so similar - is undefined, from:

function createMacro(macro, options = {}) {
  if (options.configName === 'options') {
    throw new Error(`You cannot use the configName "options". It is reserved for babel-plugin-macros.`);
  }

  macroWrapper.isBabelMacro = true;
  macroWrapper.options = options;
  return macroWrapper;

  function macroWrapper(args) {
    console.log('>>> ', args)
    const {
      source,
      isBabelMacrosCall
    } = args;

    if (!isBabelMacrosCall) {

I'd like to inspect the line of code you're referring to (which appears inside function applyMacros({) but I can't get a debugger to stop there, at least running the start script.

I tried adding a console.log() near, but nothing. It seems to be executed during Babel, but I can't really say that.

@watery
Copy link

watery commented Jan 22, 2021

I just meant for debugging, but I guess you figured it out

I misread. For the client side debugging, I'm running the start script and I've added some breakpoints within the browser debugger.

For the server side, I've edited \node_modules\react-scripts\bin\react-scripts.js , from:

if (['build', 'eject', 'start', 'test'].includes(script)) {
  const result = spawn.sync(
    process.execPath,
    nodeArgs
      .concat(require.resolve('../scripts/' + script))
      .concat(args.slice(scriptIndex + 1)),

to (see the arrow):

if (['build', 'eject', 'start', 'test'].includes(script)) {
  const result = spawn.sync(
    process.execPath,
    nodeArgs.concat(['--inspect'])                          // <---- addition
      .concat(require.resolve('../scripts/' + script))
      .concat(args.slice(scriptIndex + 1)),

This causes the debugger to kick in during script execution:

Debugger listening on ws://127.0.0.1:9229/2c7b3106-02f5-438f-8fae-004549b107bd
For help, see: https://nodejs.org/en/docs/inspector
Debugger attached.
i 「wds」: Project is running at http://10.97.236.132/
i 「wds」: webpack output is served from
i 「wds」: Content not from webpack is served from <...>\public
i 「wds」: 404s will fallback to /
Starting the development server...

and lets me open the Chrome Node-specific DevTools and see the log there too. I don't know if this is the proper way of launching the debugger in this case, I used it once some time ago.

But no breakpoint seems to be triggered on that side now. Neither when running the build script (but I need to setup a production environment to check if the issue appears there too).

@watery
Copy link

watery commented Jan 22, 2021

Side note: I tried moving the preval.macro dependency from the installed component (MyCompo) (where I specified it under peerDependencies) to the application (MyApp), but it yielded the same error.

@watery
Copy link

watery commented Jan 22, 2021

Should I add an explicit configuration to MyApp? I'm reading through the docs config section but I don't understand what to config.

@conartist6
Copy link
Collaborator

No, create-react-app specifies its babel configuration programatically (ugh). Now the lines in question have changed, but my question remains the same. You tell me that you have a failure on the line that checks isBabelMacrosCall, but isBabelMacrosCall is a property of the object. That means there must be some object, because trying to read the property isBabelMacrosCall from undefined would error on the line before. So what is it that you're trying to use as a macro?

@watery
Copy link

watery commented Jan 22, 2021

So what is it that you're trying to use as a macro?

I'm using the preval.macro package to run module.exports = new Date() inside the constructor on my own object (please see MyCompo constructor in my earlier comment);

Then that component is a dependecy of the React application that is throwing this error when I open the page that contains the above-mentioned component.

You tell me that you have a failure on the line that checks isBabelMacrosCall, but isBabelMacrosCall is a property of the object. That means there must be some object, because trying to read the property isBabelMacrosCall from undefined would error on the line before.

This is what I see inside the debugger (current line is 60):

bpm-issue87-01

@conartist6
Copy link
Collaborator

So the answer to my question is ["module.exports = new Date()"]. The question is: how the heck did that get there?

@conartist6
Copy link
Collaborator

The only way it can have gotten there is if the macro actually didn't evaluate at all, meaning that the runtime is now attempting to execute the macro evaluation code, which should never happen.

@watery
Copy link

watery commented Jan 22, 2021

Oh, and macrowrapper is called from my constructor:

bpm-issue87-02

The question is: how the heck did that get there?

I don't get that. Are you saying that there should be no text at all, or that it should be something different than a string?

My component is npm run build with the following babel.config.js file

  const presets = [
    [
      "@babel/env",
      {
        "targets": {
          "edge": "17",
          "firefox": "60",
          "chrome": "67",
          "safari": "11.1",
        },
        "useBuiltIns": "usage",
        "corejs": 3,
      }
    ],
    "@babel/preset-react"
  ];
  
  const plugins = [
    "@babel/plugin-proposal-class-properties"
  ];

  module.exports = { presets, plugins };

The only way it can have gotten there is if the macro actually didn't evaluate at all, meaning that the runtime is now attempting to execute the macro evaluation code, which should never happen.

Mmh ok, maybe there's a missing step somewhere.

@watery
Copy link

watery commented Jan 22, 2021

And this is a portion of the package.json file:

 "main": "lib/index.js",
  "directories": {
    "lib": "lib"
  },
  "scripts": {
    "build": "babel index.js index.style.js --out-dir lib --copy-files",

@conartist6
Copy link
Collaborator

Well you would certainly need babel-plugin-macros to be specified in the plugins for your component... -_-

@watery
Copy link

watery commented Jan 22, 2021

Ha! Of couse.
That component started from within MyApp, I didn't think about that.

But would this mean that that macro will be evaluated at MyCompo build time?
I need it to be evaluated at MyApp build time instead.

@conartist6
Copy link
Collaborator

conartist6 commented Jan 22, 2021

That's not how CRA works though. It has two babel configs, one for its own files, and one for deps. While it compiles macros for its own files, it makes no assumptions about code in dependencies, other than that they are valid ES.

@conartist6
Copy link
Collaborator

I'm going to close this issue because the problem it describes was fixed a while ago and now it seems we've resolved your issue.

@conartist6
Copy link
Collaborator

Out of curiosity, why would you want that to be evaluated at the App's build time? If the component has a build time I'd expect it to be when the component was built, not when the app using the component was built.

@watery
Copy link

watery commented Jan 22, 2021

Thanks for the support, I'll check the links in your last comments.

I'm investigating a way to handle app updates in a consistent way - actually some users see the updates, some don't and do not even after a page refresh (only a refresh bypassing the cache works, but it really is unfriendly for the average user).

The app build time is a prerequisite. Moving the component out in its own package will make it easily reusable among projects.

@watery
Copy link

watery commented Jan 23, 2021

@conartist6 Since you have an understanding of how CRA is configured, may I ask you a couple more questions? I've checked the files you linked and I'm trying to enable the plugin for the dependencies too.

I added require('babel-plugin-macros') in node_modules\babel-preset-react-app\dependencies.js inside the plugins property (much like it's done in create.js) but I don't see any difference. In fact I even tried mispelling require, to be sure it is being picked up, but that didn't trigger any error:

          exclude: ['transform-typeof-symbol'],
        },
      ],
    ].filter(Boolean),
    plugins: [
	  requzire('babel-plugin-macros'),                // <---- I was expecting an error here
      // Disabled as it's handled automatically by preset-env, and `selectiveLoose` isn't
      // yet merged into babel: https://github.com/babel/babel/pull/9486

Am I doing it the wrong way?

@conartist6
Copy link
Collaborator

I would strongly recommend that you check out the sources for create-react-app if you haven't already. Github's search is orders of magnitude worse than what you can do from and IDE pointed at the codebase itself. For example on Github I couldn't figure out where that dependencies config was used, but locally I discovered that it's here, which means that one possibility for why you aren't seeing your changes is the caching.

That said, CRA sounds like a terrible fit for the level of control you want. CRA is made for novices where the primary goal is "don't let them hurt themselves". How it ever became a choice for corporate products with arbitrary requirements and advanced interoperability needs beats me.

JackHowa pushed a commit to JackHowa/import-lazy-react-checker that referenced this issue Mar 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants