roact Wrap lifecycle hooks in NoYield blocks?

This was discussed in Discord, but after I thought about it some more I think it should be given some more thought. Roact will throw very strange, very hard to debug errors if you yield in any lifecycle hook. This originally came up in a bug in roact-material: https://github.com/AmaranthineCodices/roact-material/issues/25

Should NoYield be used to enforce the (currently unstated) expectation that no lifecycle hook (or render, for that matter) yields?

Mar 24 '18 15:03 AmaranthineCodices

I think this is a great idea for debugging at least.

I'm concerned about the overhead for creating coroutines every time we need to invoke render or any lifecycle method, since that's something that's pretty sensitive.

Mar 24 '18 22:03 LPGhatguy

Maybe we could conditionally enable it in GlobalConfig?

Mar 24 '18 23:03 AmaranthineCodices

Since NoYield is now much better about reporting coroutine stack traces, I think this could use some more thought - yielding in lifecycle hooks is still a Very Bad Thing:tm:!

My current thoughts on this:

Conditionally enable it via GlobalConfig; by default it is disabled.
Aggressively wrap every piece of user-supplied code in NoYield blocks when the config option is enabled.
Be sure to explain why yielding in lifecycle hooks is a Very Bad Thing:tm:.

May 15 '18 01:05 AmaranthineCodices