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

RFC - babel-macros #2730

Closed
threepointone opened this Issue Jul 6, 2017 · 62 comments

Comments

Projects
None yet
@threepointone

threepointone commented Jul 6, 2017

Code modifiers are becoming popular in the javascript ecosystem, via babel-plugins (eg - many css-in-js libs, relay, etc). This issue is to open discussion on a generic system to include them in CRA apps, but still honouring the zero-config philosophy.

The proposal here is to have a constrained form of babel-plugins, disguised to appear like regular libraries. A short writeup on what it would look like here - https://github.com/threepointone/babel-macros/blob/master/README.md

  • modifiers/'macros' would be isolated in files with .macros.js suffixes
  • there would no real trace of them in the final bundle stripping imports of all *.macros.js
  • considering they're localised and used on demand, I expect them to have little effect on build performance, so can be enabled in both dev/prod
  • because there's no dependence on webpack, it should still work with other js tooling like jest, etc.

I'm looking for feedback on the same before I start writing code for it, as well as possible first implementations (maybe a simple css-in-js lib?)

If this doesn't belong here, feel free to close the issue. Thanks!

@threepointone

This comment has been minimized.

threepointone commented Jul 6, 2017

a real world example - a theoretical, simpler port of glam would look like this -

  • npm install glam.macros
// app .js
import css from 'glam.macros'

let red = css` display: flex; color: red; `

React.render(<div className={red}>
  red text!
</div>, window.app)

would compile down to

let red = require('glam.macros/runtime')(() => 
  ['.css-7asd4a { display:flex; color: red; }']
)

React.render(<div className={red}>
  red text!
</div>, window.app)

during runtime, it would render html that looks like

<div class='css-7asd4a'>
  red text!
</div>

and add css to a stylesheet

.css-7asd4a { 
  display:flex; 
  color: red; 
}

additionally, we could also extract the css to a real .css file and automatically import them (leveraging CRA's webpack css loader automatically), and all the other goodies from glam/emotion.


another example would be relay.

instead of -

import Relay from 'relay' 
// ...

Relay.QL`...`

you'd split the two as

import Relay from 'relay'
import QL from 'relay.macros'
// ...

QL`...` 

but it would still generate code similar/same to relay's own plugin

@threepointone

This comment has been minimized.

threepointone commented Jul 6, 2017

Another usecase - integrating @kentcdodds' preval lib https://github.com/kentcdodds/babel-plugin-preval to inline node.js code into client side code for easily inlining file contents, etc without using a custom webpack loader(!)

@EnoahNetzach

This comment has been minimized.

Contributor

EnoahNetzach commented Jul 6, 2017

Cool and super-useful!

A curious question: would this mechanism be enabled on npm module macros only?
I would kinda like to be able to write a macro in the project itself without having to publish it, although I know this potentially breaks the CRA philosophy.

@kentcdodds

This comment has been minimized.

Contributor

kentcdodds commented Jul 6, 2017

I think the point would be that you just import the macros from anywhere and it would apply itself on the file you're importing. I can definitely see how this could be accomplished using a babel plugin with 0 configuration on the part of the end user. If there's interest in this, I'll build it (because it would be SO FUN!)

@suchipi

This comment has been minimized.

suchipi commented Jul 6, 2017

So something with a name like babel-plugin-macros that looks for imports/requires of /.*\/macros/, and when it finds them, imports a babel plugin from the macros lib (eg require("glam.macros/plugin"), then applies it to the Program that contained the import/require?

Sounds awesome

@kentcdodds

This comment has been minimized.

Contributor

kentcdodds commented Jul 6, 2017

Yep, that's what I was thinking @suchipi 😄

@threepointone

This comment has been minimized.

threepointone commented Jul 6, 2017

Not to the whole program, but to the specific call sites.

@kentcdodds

This comment has been minimized.

Contributor

kentcdodds commented Jul 6, 2017

The plugin would be responsible for what it does I think. Most of the time it would just find the call sites. Could make a helper to make finding call sites easier, but would want the flexibility of the whole program. I'm busy tonight, but can probably start on this tomorrow...

@threepointone

This comment has been minimized.

threepointone commented Jul 6, 2017

That constraint is important, hence the proposal. Else it's pretty unrestrained and you'll have all the problems of the babel plugin ecosystem (and why this was called macros in the first place, from lisp)

This is the basic sketch https://github.com/threepointone/babel-macros/blob/master/README.md

@suchipi

This comment has been minimized.

suchipi commented Jul 6, 2017

Ah, so it would only work for function calls, tagged template strings, and JSX, and could only modify the node where it was "called". That's an important distinction.

Which "problems of the babel ecosystem" are you trying to avoid?

@threepointone

This comment has been minimized.

threepointone commented Jul 6, 2017

2 big ones are plugin ordering, and implicit changes (you won't be able to tell what changes will happen on a source file just by looking at it)

@threepointone

This comment has been minimized.

threepointone commented Jul 6, 2017

If you want wholesale babel plugins and/or loaders, the option to eject remains, and is likely a better option.

@kentcdodds

This comment has been minimized.

Contributor

kentcdodds commented Jul 6, 2017

Oh, now I finally understand your babel-macro thing. Makes total sense. I hope nobody works on this before I get a chance to tomorrow. My mind is racing with ideas 😀

@kentcdodds

This comment has been minimized.

Contributor

kentcdodds commented Jul 6, 2017

But if course I'm just kidding 😉 if someone else wants to build this, I can't stop you 😀

@suchipi

This comment has been minimized.

suchipi commented Jul 6, 2017

Sounds a lot like OCaml extension points, which was the result of people saying "letting them change the whole parser makes things too confusing".

Do you think @decorator support would be a good idea, too?

@threepointone

This comment has been minimized.

threepointone commented Jul 6, 2017

Yeah I think decorators could work too.

@suchipi

This comment has been minimized.

suchipi commented Jul 6, 2017

Sounds super cool

@gaearon

This comment has been minimized.

Member

gaearon commented Jul 7, 2017

(We won't support decorators for this before their proposal advances. But in theory yes.)

@gaearon

This comment has been minimized.

Member

gaearon commented Jul 7, 2017

As for idea itself, heh, I like it. If @kentcdodds (or somebody else) can make a proof of concept and demonstrate it is enough to get Relay and some CSS-in-JS libraries working, I‘m game. The only thing I don’t quite like is the .macros naming, maybe we can come up with an alternative suffix.

@kentcdodds

This comment has been minimized.

Contributor

kentcdodds commented Jul 7, 2017

Alright, I found 30 minutes and I've got something! Contributions welcome :) I even published it: babel-macros. See the tests.

Ok, now I've gotta run, but this is awesome. Looking forward to some PRs! Even more test cases would be cool (just make your test an object and add skip: true)

@threepointone

This comment has been minimized.

threepointone commented Jul 7, 2017

The only thing I don’t quite like is the .macros naming, maybe we can come up with an alternative suffix.

The ideal solution would be to have separate syntax, e.g. importMacros, but that's likely never going to happen
Another option is to use a webpack inspired 'babel-macros?css', but that straight up looks alien
Alternate words off the top of my head - mods, plugs, socks(?), defines
(I'm no good at naming stuff haha)

@kentcdodds

This comment has been minimized.

Contributor

kentcdodds commented Jul 7, 2017

I want to make sure that existing tooling will work as much as I can so I want to avoid anything too weird looking... 🤔

I don't mind macros myself, but happy to consider other names 👍

@gaearon

This comment has been minimized.

Member

gaearon commented Jul 7, 2017

I want to make sure that existing tooling will work as much as I can so I want to avoid anything too weird looking...

The thing is, it also need to be explicit enough that you wouldn’t mistake it from a regular import. Because otherwise it’ll be a debugging nightmare for the person who doesn’t know what’s going on.

@suchipi

This comment has been minimized.

suchipi commented Jul 7, 2017

Flow has import type, and sweet.js uses import ... for syntax. Is adding a similar syntax extension to babylon going too far for something like this?

@kentcdodds

This comment has been minimized.

Contributor

kentcdodds commented Jul 7, 2017

Should we move discussion over to babel-macros?

I'd be fine with it if babylon supported it via a plugin, then people could just include a preset we make (and CRA could include it in its own config theoretically). I like:

import macros glam from 'glam.macros'

But I'm open to pretty much anything else.

FYI, babel-macros now supports:

  1. import
  2. require
  3. asTag
  4. asFunction
  5. asJSX

See the tests and snapshots...

Oh, heck, saved you a click 😄

screen shot 2017-07-07 at 11 10 47 am

@threepointone

This comment has been minimized.

threepointone commented Jul 7, 2017

Terrific progress, eager to hack on and use this for real stuff

gaearon added a commit that referenced this issue Jan 13, 2018

add experimental babel-plugin-macros support (#3675)
* add experimental babel-plugin-macros support

closes #2730

This will remain undocumented until the brave have tried it in the wild.

**Test Plan:**

There's currently no established way to test changes to
`babel-preset-react-app`. But I did create
[`unmaintained-react-scripts-babel-macros`](https://www.npmjs.com/package/unmaintained-react-scripts-babel-macros)
[a while back](#2730 (comment))
and it worked well.

* Pin the version

Timer added a commit that referenced this issue Jan 14, 2018

add experimental babel-plugin-macros support (#3675)
* add experimental babel-plugin-macros support

closes #2730

This will remain undocumented until the brave have tried it in the wild.

**Test Plan:**

There's currently no established way to test changes to
`babel-preset-react-app`. But I did create
[`unmaintained-react-scripts-babel-macros`](https://www.npmjs.com/package/unmaintained-react-scripts-babel-macros)
[a while back](#2730 (comment))
and it worked well.

* Pin the version

gaearon added a commit that referenced this issue Jan 14, 2018

add experimental babel-plugin-macros support (#3675)
* add experimental babel-plugin-macros support

closes #2730

This will remain undocumented until the brave have tried it in the wild.

**Test Plan:**

There's currently no established way to test changes to
`babel-preset-react-app`. But I did create
[`unmaintained-react-scripts-babel-macros`](https://www.npmjs.com/package/unmaintained-react-scripts-babel-macros)
[a while back](#2730 (comment))
and it worked well.

* Pin the version

gaearon added a commit that referenced this issue Jan 14, 2018

add experimental babel-plugin-macros support (#3675)
* add experimental babel-plugin-macros support

closes #2730

This will remain undocumented until the brave have tried it in the wild.

**Test Plan:**

There's currently no established way to test changes to
`babel-preset-react-app`. But I did create
[`unmaintained-react-scripts-babel-macros`](https://www.npmjs.com/package/unmaintained-react-scripts-babel-macros)
[a while back](#2730 (comment))
and it worked well.

* Pin the version

gaearon added a commit that referenced this issue Jan 14, 2018

add experimental babel-plugin-macros support (#3675)
* add experimental babel-plugin-macros support

closes #2730

This will remain undocumented until the brave have tried it in the wild.

**Test Plan:**

There's currently no established way to test changes to
`babel-preset-react-app`. But I did create
[`unmaintained-react-scripts-babel-macros`](https://www.npmjs.com/package/unmaintained-react-scripts-babel-macros)
[a while back](#2730 (comment))
and it worked well.

* Pin the version

Timer added a commit to Timer/create-react-app that referenced this issue Jan 14, 2018

add experimental babel-plugin-macros support (facebook#3675)
* add experimental babel-plugin-macros support

closes facebook#2730

This will remain undocumented until the brave have tried it in the wild.

**Test Plan:**

There's currently no established way to test changes to
`babel-preset-react-app`. But I did create
[`unmaintained-react-scripts-babel-macros`](https://www.npmjs.com/package/unmaintained-react-scripts-babel-macros)
[a while back](facebook#2730 (comment))
and it worked well.

* Pin the version

gaearon added a commit that referenced this issue Jan 14, 2018

add experimental babel-plugin-macros support (#3675)
* add experimental babel-plugin-macros support

closes #2730

This will remain undocumented until the brave have tried it in the wild.

**Test Plan:**

There's currently no established way to test changes to
`babel-preset-react-app`. But I did create
[`unmaintained-react-scripts-babel-macros`](https://www.npmjs.com/package/unmaintained-react-scripts-babel-macros)
[a while back](#2730 (comment))
and it worked well.

* Pin the version

Timer added a commit to Timer/create-react-app that referenced this issue Jan 15, 2018

add experimental babel-plugin-macros support (facebook#3675)
* add experimental babel-plugin-macros support

closes facebook#2730

This will remain undocumented until the brave have tried it in the wild.

**Test Plan:**

There's currently no established way to test changes to
`babel-preset-react-app`. But I did create
[`unmaintained-react-scripts-babel-macros`](https://www.npmjs.com/package/unmaintained-react-scripts-babel-macros)
[a while back](facebook#2730 (comment))
and it worked well.

* Pin the version
@gaearon

This comment has been minimized.

Member

gaearon commented Jan 18, 2018

Fixed by #3675.

You can start testing it with the first 2.x alphas. Instructions are here: #3815

@gaearon gaearon closed this Jan 18, 2018

viankakrisna added a commit to viankakrisna/create-react-app that referenced this issue Jan 22, 2018

add experimental babel-plugin-macros support (facebook#3675)
* add experimental babel-plugin-macros support

closes facebook#2730

This will remain undocumented until the brave have tried it in the wild.

**Test Plan:**

There's currently no established way to test changes to
`babel-preset-react-app`. But I did create
[`unmaintained-react-scripts-babel-macros`](https://www.npmjs.com/package/unmaintained-react-scripts-babel-macros)
[a while back](facebook#2730 (comment))
and it worked well.

* Pin the version

@stereobooster stereobooster referenced this issue Jan 29, 2018

Open

Roadmap #73

3 of 3 tasks complete

akstuhl added a commit to akstuhl/create-react-app that referenced this issue Mar 15, 2018

add experimental babel-plugin-macros support (facebook#3675)
* add experimental babel-plugin-macros support

closes facebook#2730

This will remain undocumented until the brave have tried it in the wild.

**Test Plan:**

There's currently no established way to test changes to
`babel-preset-react-app`. But I did create
[`unmaintained-react-scripts-babel-macros`](https://www.npmjs.com/package/unmaintained-react-scripts-babel-macros)
[a while back](facebook#2730 (comment))
and it worked well.

* Pin the version

facebook-github-bot added a commit to facebook/relay that referenced this issue Aug 13, 2018

Support babel-plugin-macros (#2171)
Summary:
This PR adds support for [babel-plugin-macros](https://github.com/kentcdodds/babel-plugin-macros).

Usage:
```js
import graphql from 'babel-plugin-relay/macro';
// Or:
// const graphql = require('babel-plugin-relay/macro');
```

`graphql` is exported from both the `react-relay` and `relay-runtime` packages, but as I created this PR mostly in the interest of possible Create React App integration, I only added an export to `react-relay`.

This PR currently only adds support for Relay Modern. I'm not sure if Compat and/or Classic should be supported?

I currently only have a single basic test. I certainly could copy over the fixtures in `__tests__/fixtures-modern` and change the imports, although I'm not sure how beneficial that would be, as most of the logic is shared with `babel-plugin-relay`.

Closes #1968

Related: facebook/create-react-app#2730 (comment)

/cc kentcdodds
Pull Request resolved: #2171

Reviewed By: jstejada

Differential Revision: D6831205

Pulled By: kassens

fbshipit-source-id: 704a239b45974359cc1dcecacd85efb4aeddeef5
@gaearon

This comment has been minimized.

Member

gaearon commented Oct 2, 2018

@kentcdodds

This comment has been minimized.

Contributor

kentcdodds commented Oct 2, 2018

HUGE Thank you to @threepointone for the idea 👏👏👏👏

@coodoo

This comment has been minimized.

coodoo commented Oct 2, 2018

Out of curiosity, is it possible to use macro to modify webpack config file? Say if I want to add a new loader into the config without ejecting? Thanks.

cc @gaearon @kentcdodds

@tricoder42

This comment has been minimized.

tricoder42 commented Oct 2, 2018

@coodoo Nope, it's not possible. Macros only transform AST nodes. There're like Babel plugins.

@threepointone

This comment has been minimized.

threepointone commented Oct 2, 2018

I don’t think CRA runs their webpack config through their own babel pipeline but hahaha nice try 😆

@threepointone

This comment has been minimized.

threepointone commented Oct 2, 2018

HUGEST Thank you to @kentcdodds for actually building the thing (based on a hasty gist lol) and being such a good steward! You’re the best.

@coodoo

This comment has been minimized.

coodoo commented Oct 2, 2018

Oh snap, so is there anyway other than ejecting that could allow one to add new loaders to the webpack config? 😂

@threepointone

This comment has been minimized.

threepointone commented Oct 2, 2018

you could write a macro that reads the file, parses/transforms it, and inlines it at the macro site call. eg -

import csvToJs from 'csvToJs.macro'

let data = csvToJs('./myfile.csv') // will inline the contents of myfile.csv as a js object here

a disadvantage of this approach is that if you require it in a different file, it'll get inlined again, increasing the size of your bundle for no good reason.

another approach is to write the contents of the file to another js file (ie - in the macro, you'd call fs.writeFileSync to a 'myfile.csv.js' file, and convert -

let data = csvToJs('./myfile.csv')

to

let data = require('./myfile.csv.js')

but this would mean you'd generate spurious files in your source folder (or whichever temp folder you output to).

either option may work for you, depending on your constraints.

@kentcdodds

This comment has been minimized.

Contributor

kentcdodds commented Oct 2, 2018

but this would mean you'd generate spurious files in your source folder (or whichever temp folder you output to).

I suggest writing the file to node_modules/.cache/some-namespace/path/to/file.js then you don't need to worry about generating spurious files :)

I think that there is room for someone to make a module that makes doing this with macros very easy:

const {createMacro} = require('babel-plugin-macros')
const createTempFileForMacro = require('the-module-someone-writes-to-make-this-easy')

module.exports = createMacro(csvToJsMacro)

function csvToJsMacro({references}) {
  // do stuff
  createTempFileForMacro(someRelevantArguments)
  // do more stuff
}

I think many macros would benefit from something like this (though I don't believe it's something that belongs in the core of babel-plugin-macros).

@satya164

This comment has been minimized.

Contributor

satya164 commented Oct 3, 2018

One problem with Babel macro reading a file is that unless the file which uses the macro changes, it won't be called again due to the Babel loader cache, which can be very annoying.

@Timer

This comment has been minimized.

Collaborator

Timer commented Oct 3, 2018

@satya164 this problem does not exist in Create React App. There is continuous cache invalidation for files using macros.
Clearing the cache manually should be avoided.

@satya164

This comment has been minimized.

Contributor

satya164 commented Oct 3, 2018

Timer added a commit to Timer/babel-plugin-macros that referenced this issue Oct 3, 2018

Add CRA note to caveats
facebook/create-react-app#2730 (comment)

We don't want users thinking they have to purge the cache, as this will be very detrimental to their rebuild speed.

kentcdodds added a commit to kentcdodds/babel-plugin-macros that referenced this issue Oct 3, 2018

docs: add CRA note to caveats (#83)
facebook/create-react-app#2730 (comment)

We don't want users thinking they have to purge the cache, as this will be very detrimental to their rebuild speed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment