Create-react-app: Re-enable babel-plugin-transform-react-constant-elements when it’s not buggy

[gaearon]

See #525 for the reference.

[gaearon]

Here’s a list of bugs I found by searching. I think it’s on us to triage and fix them.

transform-react-constant-elements

• [x] https://github.com/babel/babel/issues/3915
• [x] https://github.com/babel/babel/issues/4106
• [x] https://github.com/babel/babel/issues/4223
• [x] https://github.com/babel/babel/issues/4324
• [x] https://github.com/babel/babel/issues/4419
• [x] https://github.com/babel/babel/issues/4458
• [x] https://github.com/babel/babel/issues/4516
• [x] https://github.com/babel/babel/issues/4804
• [x] https://github.com/babel/babel/issues/4397
• [x] https://github.com/babel/babel/issues/5149
• [ ] babel/babel#5325
• [ ] babel/babel#5315

transform-react-inline-elements

• [x] https://github.com/babel/babel/issues/3728
• [x] https://github.com/babel/babel/issues/4027
• [x] https://github.com/babel/babel/issues/4455

[DrewML]

Friendly update: I have a PR in to fix babel/babel#3728. Additionally, babel/babel#4455 doesn't appear to be an issue in the current version (based on my testing with the info provided in the issue). I'll be working on babel/babel#4027 pretty soon so that we can get transform-react-inline-element back into a usable state for y'all.

Once those are done, I'll start trying to look into the pile of react-constant-elements bugs 😑

[gaearon]

Wow, thank you!

[DrewML]

Alright, have a PR in for babel/babel#4027 now, too. Once those 2 PRs get merged, you should be ready to re-enable that plugin 😸

[DrewML]

These one's can be checked off for transform-react-constant-elements:

• babel/babel#3915
• babel/babel#4106
• babel/babel#4223

It turns out Sebastian fixed these here in July, and the issues just never got closed out.

Few more to go 😃

[DrewML]

OK, after the release today, all the bugs related to transform-react-inline-elements (that we know of) have been addressed. Please reach out if you see more once it is re-enabled.

[appden]

FYI: I just submitted PR babel/babel#4940 that fixes babel/babel#4419 and babel/babel#4804 (which should be added to the list above, and babel/babel#4458 should be updated as closed).

There's an existing PR (babel/babel#4787) to address babel/babel#4397, but it looks stalled on implementation details.

[Timer]

Neat, @appden! I've added https://github.com/babel/babel/issues/4804 to the list.

I don't want to check off https://github.com/babel/babel/issues/4458 (yet) because it seems to have been "magically" fixed without a trace of where the regression was introduced or resolved. We can save that one for last and determine if we want to let it slide.
I would love to see a test case added which prevents future regressions for that specific issue.
I'm not sure on other's opinions about this.

[slorber]

@gaearon you might want to track this one too: https://github.com/babel/babel/issues/5149

[gaearon]

Thanks, added to the list.

[gaearon]

And... we're good? 🤗

[gaearon]

With next Babel release that is.

[slorber]

@gaearon there might be other bugs discovered:

• https://github.com/babel/babel/issues/5325
• https://github.com/babel/babel/issues/5315

Maybe it's worth waiting a bit for community feedback after the fixes, and only reactivate this in CRA when it looks stable?

[gaearon]

👍

[brantphoto]

left a comment in babel/babel/issues/5325, for me I had to make sure I to make sure that babel-plugin-transform-react-constant-elements was the last plugin listed in order for it to work without error

[slorber]

Adding the constant-elements plugin last solved the problem for me too

[gaearon]

Wow, thanks for the info. Seems like we have to wait for 7.0 for https://github.com/babel/babel/pull/5415 though.

[slorber]

Some other issues related to the plugin have been opened since: https://github.com/babel/babel/search?q=transform-react-constant-elements&type=Issues&utf8=%E2%9C%93

[gaearon]

Postponing as issues just keep coming up. 😞
I’m not sure we’ll ever enable it.

[tarjei]

Maybe you can "prepack" it?
man. 8. mai 2017 kl. 16.36 skrev Dan Abramov [e-mail]:

Postponing as issues just keep coming up. 😞
I’m not sure we’ll ever enable it.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/facebookincubator/create-react-app/issues/553#issuecomment-299884976,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAM5PwARtHPy0KDuvSZsRI1hya_qU6KFks5r3yh_gaJpZM4J0Fxp
.

>

---

Tarjei Huse
Mobil: 920 63 413

[levithomason]

Looks like the last two unchecked items on the list are closed issues:

babel/babel#5325
babel/babel#5315

Are we good to go here now?

[gaearon]

I think they're waiting for Babel 7.

[gaearon]

Honestly I don't see this happening. New bugs keep reappearing, and I expect this will continue to be true unless Babel's architecture gets overhauled. The plugin itself is also not the best solution because it moves the cost to initialization time, potentially hurting the TTI. An ideal solution would only initialize what's necessary on demand.

I think we won't proceed with this. In the future (one-two years) I expect we can revisit a similar optimization utilizing Prepack instead.

[ggregoire]

What about babel-plugin-transform-react-inline-elements? Any reason to not enabling this plugin?