pmndrs / react-spring

✌️ A spring physics based React animation library
http://www.react-spring.dev/
MIT License
28k stars 1.19k forks source link

React-Spring breaks when compiled to ES5 #592

Closed ShanonJackson closed 5 years ago

ShanonJackson commented 5 years ago

I spent about 5 hours tracking down the issue, yesterday.

When React-Spring is compiled through my ts-loader or awesome-typescript-loader targetting commonjs and ES5, when i import that build into another project react-spring won't animate.

When React-Spring is compiled through ts-loader or awesome-typescript-loader targetting commonjs and ES6, when i import that build into another project react-spring will animate.

Iv'e tried to track down the exact line the transpiling breaks on, but i can't find it because there's no errors in console and i'm not too familiar with the react-spring codebase.

Can you please fix this by compiling react-spring down to ES5 rather than ES6 yourself, run the tests then build?

drcmda commented 5 years ago

that doesn't give me lots to work with tbh. the lib is getting bundled up and bablified to env ">1%, not dead, not ie 11, not op_mini all". and used in all kinds of build systems and production code, including es5+ie11 (via a second babel pass). to me it sounds like an issue with your build system. if you have a reduced reproducable case + a repo for it, if i have time i could take a look.

drcmda commented 5 years ago

PS. i remember i ran into one such case myself once with docz. The build didn't animate, dev worked fine. Turned out there was some weird issue in webpack having to do with sideeffects and treeshaking, it was ripping out the globals section from this lib, where the target defines what a "div" is, what colors are, and how to write styles, leaving only a clueless interpolator. I fixed it by switching off treeshaking, which seemed to be broken.

https://github.com/react-spring/react-spring/blob/d415cfb853356a344d36dff76fa1fdf6610fe5a3/doczrc.js#L148-L151

You could try it, who knows.

ShanonJackson commented 5 years ago

Im confident i can reproduce.

I doubt the webpack tree shaking is causing it because when i swap ts-loader and awsome-typescript-loader out for babel loader and turn off type checking it compiles down to ES5 correctly and also runs the animations. Which leads me to believe that how typescript transpiles your ES6 classes to ES5 has broken the functionality in some way.

I will try make a minimum viable repro now.

EDIT: give me 5mins

drcmda commented 5 years ago

if thats the case i think the ticket should go to the typescript github. my time is very limited these days due to some ongoing projects.

ShanonJackson commented 5 years ago

got a working reproduce now. EDIT: just putting it on a public repo

ShanonJackson commented 5 years ago

https://github.com/ShanonJackson/reactspringbug

let me know if you have any problems in setup at all

setup: npm install npm install -g http-server http-server ./static go to localhost:8080 notice height/width wont animate. change target in tsconfig to "es6" npm run build hard refresh cache localhost:8080 to get new file, height/width animates

This bug is currently blocking my production build of my application.

EDIT: I think the best suggested fix is for react-spring to compile down to ES5

PooLP commented 5 years ago

Hi,

I don't have same problem, but it's related to ES5 / ES6 compilation

I'm use a Microsoft Framework to create a SharePoint WebPart (SPFx). The target is only ES5 and I can not change it for compatibility purpose. I don't access to Webpack config..

If i'm use a 8.x version, the build operation crash with UglifyJs error : Unexpected token: name (ParallaxLayer)

If i'm use a 7.x version, build work perfectly.

ShanonJackson commented 5 years ago

@PooLP try change your react-spring/renderprops imports to react-spring/renderprops.cjs

PooLP commented 5 years ago

I have tested but i have another error : Unexpected token: name (bugfixes)

roberteles commented 5 years ago

I have tested but i have another error : Unexpected token: name (bugfixes)

I managed to get it build by using .cjs at the end of the import like this: import { Transition } from 'react-spring/renderprops.cjs'

PooLP commented 5 years ago

I tested but I still have the error. The 7.x of react-spring lib work fine.

ShanonJackson commented 5 years ago

Thanks, yes it is true the build in my example does work when swapped to .cjs however it should work when the ES6 code is transpiled to ES5 through me.

drcmda commented 5 years ago

Has anyone managed to track it down to anything specific? I'm not good with typescript, and these days i can't spend much time on tooling issues. If you all want to help that would be super appreciated!

ShanonJackson commented 5 years ago

I haven't managed to track it down to anything specific and don't personally have the time.

There are also several other issues which are semi-related.

import { useSpring, animated } from "react-spring" breaks when tsconfig module is commonjs because react-spring is using synthetic default imports of react (I.E import React from "react") which are causing massive issues for me, they should be import * as React from "react" which leads to less compiled output and is more inline with the specification.

import { useSpring, animated } from "react-spring" breaks by not being able to work on IE11 because there is no visible react-spring.cjs i cannot work around this because the code is inherently ES6.

Related to the above issue is that transpiling the ES6 code discussed above to ES5 through the typescript compiler targetting commonjs breaks react-spring in new and interesting ways that include animations running at 0.0000000001fps and/or not running at all.

I understand i'm not offering a pull request to fix the issues as i'm currently under the gun for alteast 2 more months at work and also not familiar with the react-spring library internals but it would be nice if some champion of the community would transpile react-spring to commonjs es5 by default and also perferably not use synthetic default importing.

@drcmda

aleclarson commented 5 years ago

import { useSpring, animated } from "react-spring" breaks when tsconfig module is commonjs because react-spring is using synthetic default imports of react (I.E import React from "react") which are causing massive issues for me, they should be import * as React from "react" which leads to less compiled output and is more inline with the specification.

Can you open a new issue (with a repro) for this?

bdrobinson commented 5 years ago

This bit our project too as we've never had to compile anything in node_modules before. For anyone else who was confused, here's how I set up babel to transform react-spring:

1. Update the JS loader in webpack. Previously we just had exclude: "node_modules", but I updated our JS loader to use include instead:

{
    test: /\.js$/,
    loader: "babel-loader",
    include: [
        path.resolve(__dirname, "src"),
        path.resolve(__dirname, "node_modules", "react-spring"),
    ],
}

2. If using babel 7, make sure you're using babel.config.js rather than .babelrc. It seems bizarre that this would make a difference but apparently babel 7 doesn't apply transforms from .babelrc to things in node_modules even if you've configured the webpack loader correctly, so you need to migrate to babel.config.js. See https://stackoverflow.com/a/54933703/968098.

Not that this super relevant to this thread, but I had to do a bit of googling to fix this so thought it might be helpful to newcomers.

jsu93 commented 5 years ago

react-spring compile es6 code to es5. Old browser users are annoying