isMacrosName doesn't provide the full path

[FezVrasta]

• babel-plugin-macros version: 3.1.0
• node version: 12.x
• npm version: 6.14

Relevant code or config

const macrosRegex = /[./]macro(\.js)?$/;
const testMacrosRegex = v =>
  macrosRegex.test(v) && v.indexOf('foobar/macro') === -1;

module.exports = {
  plugins: [
    [
      'macros',
      {
        isMacrosName: testMacrosRegex,
      },
    ],
  ],
};

File structure:

node_modules
  foobar
    index.js
    macro.js
app.js

// node_modules/foobar/index.js
import macro from './macro';

// app.js
import foobar from 'foobar';

What you did:

I tried to exclude foobar/macro from babel-plugin-macros

What happened:

The file is treated as a Babel macro.

Problem description:

The problem I'm having is with relative macro-like imports such as:

import macro from './macro';

The above code is trying to import a file named macro but that is not a Babel macro.

The custom configuration isn't able to filter out this specific import because the path provided to testMacrosRegex is ./macro rather than the absolute one (node_modules/foobar/macro).

Suggested solution:

isMacrosName should provide an "absolute-like" import (starting from the root of the project) in order to allow to filter out unwanted macro files.

Alternatively, the plugin could provide a configuration option to specify paths that should be ignored.

[conartist6]

Yeah I see your problem, but I'm not quite sure about the solution. Could you tell me a little more about your setup? You must be running babel on code inside node_modules, but I am surprised that babel would have the macros plugin enabled when transpiling code in node_modules. I would say what you're experiencing is really a very general problem: it is not safe to run arbitrary transforms on module code because unless the syntax in use is part of the ECMAScript specification you won't know how the module author expected it to be interpreted.

[conartist6]

Let's say I was transpiling for .. of loops in code. Because the spec tells us how they should behave I can safely transpile them down even in node_modules as long as both the transpiled and untranspiled code have the same semantics.

But there are multiple ways of transpiling for .. of loops: in loose mode they are simply translated to index loops on the assumption that the target of the loop is an array. But for that code to work it requires extra knowledge of the program: specifically that any for .. of loops are always over arrays. If you applied that same loose transform to all of node_modules, you would expect things to break because you've altered the meaning of the language. I think it's quite reasonable as a rule that code in node_modules must be module code executable by node, and it must be free to use any syntax (including import paths) which is valid and legal.

[conartist6]

All that said, I do think isMacrosName has clear room for improvement. I think it should be given a second argument: the path of the file the import or require statement appears in. This would retain backwards compatibility but also ensure that any valid use cases could be met. But if I am right about what your problem is it would not be the solution you need.

[FezVrasta]

Thanks for the reply, in my case I have a Create React App and I'm trying to use the @kitware/vtk.js library, that internally has a macro.js file.

@kitware/vtk.js is published with ES Modules (import/export) so in order to use it with Jest I need to put it in the transformIgnorePatterns, and this originates the error I described above.

I think the solution you described should work for my case, although CRA doesn't allow to configure the babel-plugin-macros so I would need to resort to craco.

[conartist6]

I was pretty sure CRA didn't run macros on node_modules code...

[FezVrasta]

It doesn't, but if you add transformIgnorePatterns to the Jest configuration then it seems like it starts doing it.

To be fair the best way to address this issue that comes to mind would be for the plugin to check that the imported macro is actually a Babel Macro.

[conartist6]

To be fair the best way to address this issue that comes to mind would be for the plugin to check that the imported macro is actually a Babel Macro.

That sounds nice, but really I think it would make a mess. One of the benefits of transpilation is that it's a pure 1:1 operation. Given the same input content you should always get the same output. Reading other module files to decide how transpile a particular input would mean that results of transpilation could never be properly cached. Right there you'd be breaking jest.

It doesn't, but if you add transformIgnorePatterns to the Jest configuration then it seems like it starts doing it.

Weird. And that package publishes non-esm code too so I don't see why jest would have problems. If it doesn't understand module code it shouldn't be reading the module field in package.json

[FezVrasta]

There's a bit of confusion, there are two vtk.js packages, one is a legacy package that also ships umd, one is a bundler-friendly one that doesn't ship UMD though (https://unpkg.com/browse/@kitware/vtk.js@18.6.0/).

[conartist6]

Here's my summation: since babel-plugin-macros works by "stealing" some syntaxes that would otherwise be valid JS, it must never be run on code that has not explicitly opted into being transpiled with the macros plugin. This is not a flaw, but rather an explicit design choice.

This issue seems to have been caused by a bad jest configuration. In create-react-app the default set of babel transforms includes babel-plugin-macros, and that set of transforms is configured to run only on files which are not inside node_modules. The submitter configured jest with transformIgnorePatterns: ['node_modules/(?!pgk_name/)'], where pkg_name is the name of a package which has syntactic conflicts with macros. The problem appears to be that method of reconfiguring jest, which causes it to apply a set of plugins which includes macros to module code, which is incorrect. Explicitly applying the babel-jest transform to that package should be fine though.