effector / effector

Business logic with ease ☄️
https://effector.dev
MIT License
4.61k stars 241 forks source link

`effector/babel-plugin`: gets broken by babel macro? #542

Open Kelin2025 opened 3 years ago

Kelin2025 commented 3 years ago

What is the current behavior: I made a simple babel macro which transforms this:

when($isOpened, () => {
  sample({ source: source, target: target })
  forward({ from: from, to: to })
  $store.on(smth, (prev, next) => next)
})

to

sample({ 
  source: guard({ 
    source, 
    filter: $isOpened 
  }), 
  target: target 
})

forward({ 
  from: guard({ 
    source: from, 
    filter: $isOpened 
  }), 
  to: to 
})

$store
  .on(guard({ source: smth, filter: $isOpened }), (prev, next) => next)

However, it breaks effector/babel-plugin

Stack trace ``` TypeError: /src/index.js: Cannot read properties of undefined (reading 'start') at eval (babel-plugin.js:506) at e.find (babel.7.12.12.min.js:1) at setConfigForConfMethod (babel-plugin.js:503) at fn (babel-plugin.js:49) at applyMethodParsers (babel-plugin.js:204) at e.CallExpression (babel-plugin.js:238) at n (babel.7.12.12.min.js:1) at e._call (babel.7.12.12.min.js:1) at e.call (babel.7.12.12.min.js:1) at e.visit (babel.7.12.12.min.js:1) (anonymous) @ child-handler.ts:101 l @ runtime.js:45 (anonymous) @ runtime.js:274 forEach.e. @ runtime.js:97 n @ asyncToGenerator.js:3 l @ asyncToGenerator.js:29 Promise.then (async) n @ asyncToGenerator.js:13 a @ asyncToGenerator.js:25 (anonymous) @ asyncToGenerator.js:32 (anonymous) @ asyncToGenerator.js:21 (anonymous) @ child-handler.ts:16 (anonymous) @ child-handler.ts:60 l @ runtime.js:45 (anonymous) @ runtime.js:274 forEach.e. @ runtime.js:97 n @ asyncToGenerator.js:3 a @ asyncToGenerator.js:25 (anonymous) @ asyncToGenerator.js:32 (anonymous) @ asyncToGenerator.js:21 (anonymous) @ child-handler.ts:16 (anonymous) @ child-handler.ts:28 ```

Reproduction: https://codesandbox.io/s/mystifying-currying-dl9ns?file=/src/index.js Macro source: https://github.com/Kelin2025/effector-when.macro/blob/main/src/index.js


I'm kinda new to babel plugins, so I probably did something wrong, but it works perfectly fine with other babel plugins 🤔

zerobias commented 3 years ago

This mean that babel-plugin is working too late: it’s always better to run it on unprocessed sources, as it really, really need unambiguous matching between runtime and lines of source code. It is essential for sid generation

But it shouldn’t throw errors, for sure

zerobias commented 3 years ago

Btw, very nice trick, I like it 👍👍

Kelin2025 commented 3 years ago

This mean that babel-plugin is working too late

Hm, if we change plugins order in .babelrc, it still gives the same error. Does this order even matter?

it’s always better to run it on unprocessed sources

Yeah, but macros is about codegen, so I think it's good to process generated code as well. Can plugins process modified code the same way they process the real one? 🤔 In future, we'll have a visualisation tool which rely on sources, and it'd be cool to see generated connections as well

zerobias commented 3 years ago

Yes, in general the order is always matter in babelrc

As for our use case, if one of the plugins add some code, then at the time of effector’s plugin execution, we need to restore code-to-location correspondence in some way. In other words it would act like a rendering code to string after each plugin. This looks like a heavy task to do by default, so it’s not surprising that babel just fill locations of changed code with nulls

Walking through source code being a first plugin in queue

const foo = createEvent() // => {line: 1, col: 14}

Walking through the same code but after some changes by another plugins

const foo = (() => { // => {line: 1, col: 14}
  let _tmp = createEvent() // => null
  return tmp
})()

I think that there might be a way to force babel to recalculate locations or something like that, but I don’t know how we might learn about it

Kelin2025 commented 3 years ago

Hm. Can I somehow force babel to transform code to one-liner? Now it does

guard({
  source,
  target
})

It can work with guard({ source, target }) I haven't found anything related in docs 🤔 There're only one things I found. Using replaceWithSourceString instead of replaceWith might work, but I guess that's gonna critically reduce performance