[Packager] RCTPOPAnimationManager.js - syntax error after conversion with babel

[johanneslumpe]

In Libraries/BatchedBridge/BatchedBridgedModules/POPAnimation.js the following block is invalid after transpiling it with babel:

var RCTPOPAnimationManager = require('NativeModules').POPAnimationManager;
if (!RCTPOPAnimationManager) {
  // POP animation isn't available in the OSS fork - this is a temporary
  // workaround to enable its availability to be determined at runtime.
  // For Flow let's pretend like we always export POPAnimation
  // so all our users don't need to do null checks
  module.exports = ((null: any): typeof POPAnimation);
} else {
  /*...*/
}

It will turn to

'use strict';

var RCTPOPAnimationManager = require('NativeModules').POPAnimationManager;
if (!RCTPOPAnimationManager) {
  // POP animation isn't available in the OSS fork - this is a temporary
  // workaround to enable its availability to be determined at runtime.
  // For Flow let's pretend like we always export POPAnimation
  // so all our users don't need to do null checks
  module.exports = ;
} else {
  /* ... */
}

resulting in a syntax error because of the broken moule.exports assignment.

[brentvatne]

module.exports = ((null: any): typeof POPAnimation);

is this valid syntax or is the issue with the transformer?

[brentvatne]

Ran into this myself on facebook/react-native#8b93b9 installing from https://github.com/rnplay/rnplay-ios/tree/c97ebe51860abcc249a9cbbf754b3bc8e0ddaad0

[sebmck]

Fixed via babel/babel@64f4209. This should have been reported on the babel/babel repo 😄

[brentvatne]

Thanks @sebmck!

[ide]

@amasad looks like the next step is to upgrade babel

[sebmck]

Why is the Babel version fixed 😨

[ide]

I believe it has to do with how fb internally downloads npm packages. It'd be better for the external community for fb to use semver's caret in react-native's package.json, run npm shrinkwrap to generate npm-shrinkwrap.json to pin all dependencies for internal use, and then exclude npm-shrinkwrap.json from the external directory that is synced out to GitHub. cc @vjeux would this work for the team?

[amasad]

What @ide said re npm packages. Actually, since we are currently checking in the modules internal, I don't see any harm in unpinning the dependencies. Anyways, we want to update Babel anyways, to get the new perf wins.

@DmitrySoshnikov: I believe you were starting to update babel?

[DmitrySoshnikov]

@DmitrySoshnikov: I believe you were starting to update babel?

Yep.

[browniefed]

Based on the docs Pop animation is no longer supported, deprecated in favor of Animated.
Not sure if we are concerned about the babel upgrade w/ POP no longer supported.
@brentvatne POP no longer supported.

[brentvatne]

Yeah, also we're not seeing this error anymore, thanks @browniefed