Prevent fiber code from leaking into RN stack renderer (and vice versa)#9775
Prevent fiber code from leaking into RN stack renderer (and vice versa)#9775bvaughn merged 1 commit intofacebook:masterfrom
Conversation
Since stripEnvVariables was used to replace __DEV__ references, I assumed it (and other plugins) we run before requires statements were processed. Obviously I was wrong 😬 and as a result, the RN Stack and Fiber builds were way too large. This is an attempt to mimic the approach taken with DOM renderer and stub out modules that we explicitly don't want to include. The alternative to this would have been to fork findNodeHandle, NativeMethodsMixin, ReactNativeBaseComponent, etc. and essentially avoid using the feature flag. That didn't seem tenable. The previous injection approach also doesn't work here because the circular references it resulted in caused Rollup to choke when creating the modules.
bvaughn
force-pushed
the
rn-flat-bundles-pt2
branch
from
ab2c243 to
44cf03e
Compare
|
Sorry, not sure I understand. What was referencing the modules across stack/fiber? Would it be better to make the build fail when seeing those references rather than stubbing them out? |
No, not unless we wanted to fork a dozen files to prevent stack from requiring fiber or vice versa. The problem is that RN can't rely on the same sort of injection techniques as DOM uses because Rollup chokes on the circular dependencies. And feature-flag switching isn't enough either b'c Rollup requires both branches of the conditional. I thought I could replace So the easiest fix, at least for the short term, is to do what DOM does and just black-list a few of the entry-point files. It kind of sucks but... I did consider other options and it seems the least bad. |
|
I also think it might be better to focus on reaching full parity and getting Fiber to 100% there and not spend any more effort on debt related to Stack/Fiber separation. |
5 participants
Since
stripEnvVariableswas used to replace__DEV__references, I assumed it (and other plugins) we run before requires statements were processed. Obviously I was wrong 😬 and as a result, the RN Stack and Fiber builds were way too large. This is an attempt to mimic the approach taken with DOM renderer and stub out modules that we explicitly don't want to include.The alternative to this would have been to fork
findNodeHandle,NativeMethodsMixin,ReactNativeBaseComponent, etc. and essentially avoid using the feature flag. That didn't seem tenable. The previous injection approach also doesn't work here because the circular references it resulted in caused Rollup to choke when creating the modules.A quick smoke-test of Ads Manager and Catalyst apps (both stack and fiber) seems good. Jest tests and flow also seem good. And more importantly...