Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minified bundle used for production causes arrows not to display as pretty curves #24

Closed
DeveloperAlly opened this issue Dec 5, 2018 · 21 comments

Comments

@DeveloperAlly
Copy link

Minified bundle used for production causes arrows not to display as pretty curves

These arrows look great when in dev mode. But once I build a prod (using npm build) to go to production, the arrows lose their curve and therefore look straight & warped :(

@pierpo
Copy link
Owner

pierpo commented Dec 5, 2018

Oh, oops.
I'll have a look.

If you want me to go faster, you may provide a minimal example 😊

@DeveloperAlly
Copy link
Author

DeveloperAlly commented Dec 6, 2018

@pierpo thankyou!! I will try to get one together

@zgec
Copy link

zgec commented Dec 14, 2018

Experiencing exactly the same issue here...
I am using Next.js 7.0.2 and its associated build script

I should mention, the render happens on client not on server.

I'll have a closer look tomorrow as well... Thank you so much for this utility btw, its really great (other than this bug) 😄

@zgec
Copy link

zgec commented Dec 15, 2018

I managed to fix this by cloning the repo @ master and building the lib/react-archer.js file on my own (and then including that one to my codebase instead of the package import).

After finding that this works, I decided to prettify & diff the two files (one that comes from npm and the one I built myself):

--- old-react-archer.js Sat Dec 15 12:08:38 2018
+++ react-archer.js Sat Dec 15 12:08:34 2018
@@ -1,6 +1,6 @@
 ! function(t, e) {
   "object" == typeof exports && "object" == typeof module ? module.exports = e(require("react")) : "function" == typeof define && define.amd ? define(["react"], e) : "object" == typeof exports ? exports.ReactArcher = e(require("react")) : t.ReactArcher = e(t.React)
-}(window, function(t) {
+}("undefined" != typeof self ? self : this, function(t) {
   return function(t) {
     var e = {};
 
@@ -630,11 +630,12 @@
             r.setState(function(t) {
               return {
                 fromTo: function(t, e, r) {
-                  return v(t.filter(function(t) {
+                  var n = t.filter(function(t) {
                     return !r.find(function(e) {
                       return e.from.id === t.from.id && e.to.id === t.to.id
                     })
-                  })).concat(v(e))
+                  });
+                  return [].concat(v(n), v(e))
                 }(t.fromTo, o, i)
               }
             })
@@ -775,6 +776,9 @@
               }, o.a.createElement("svg", {
                 style: m
               }, o.a.createElement("defs", null, this.generateAllArrowMarkers()), t), o.a.createElement("div", {
+                style: {
+                  height: "100%"
+                },
                 ref: this.storeParent
               }, this.props.children)))
             }

So, it seems to me that these are actually changes from recent (merged) commits that fixed this issue but havent been published to npm yet?

@pierpo
Copy link
Owner

pierpo commented Dec 16, 2018

@zgec Yes indeed! This fix was done in this PR #29, and I did not publish. I didn't know it would solve this one as well.

Will publish right now!

Very nice job at debugging it, thank you very much 😊

@pierpo
Copy link
Owner

pierpo commented Dec 16, 2018

@zgec @AlisonWonderlandApps Just published a new version. Feel free to reopen if it doesn't work 😊

Thanks again for the issue and pointing out the problem!

@pierpo pierpo closed this as completed Dec 16, 2018
@DeveloperAlly
Copy link
Author

Thankyou @pierpo !! I'm really impressed by your dedication to this library and your responsiveness to issues as well! Great job! 👍 🥇

@pierpo
Copy link
Owner

pierpo commented Jan 10, 2019

@AlisonWonderlandApps Thanks for the kind words 😊
Actually I did not fix this issue myself, @jfo84 did 😉

@geoff-va
Copy link

I am actually still running into this issue - the arrows work fine in dev mode and then once minified with npm run build, the anchors aren't on the correct sides. Everything seems to be offset and one arrow is actually pointing in the wrong direction now (back in on itself):

Production
production

Dev
dev

Highlighted box on the left is the target object.

Thanks!

@jfo84
Copy link
Contributor

jfo84 commented Jan 14, 2019

Hey @geoff-va, I can take a look at this but I don't have any immediate guesses. Seems very odd at first glance

@geoff-va
Copy link

I agree - I'm pretty new to React so am not clear on what black magic may be occurring when everything gets minified. I wonder if it has to do with render timing in someway? really odd for sure.

I appreciate you looking into this, though!

@jfo84
Copy link
Contributor

jfo84 commented Jan 16, 2019

We generate different path elements in prod vs. dev. I haven't been able to reproduce with our example-generating code, but here is an SVG from an internal build

Dev

<path d="M129.515625,120 C129.515625,0 129.515625,254.375 129.515625,254.375" marker-end="url(http://localhost:3000/builder/5beede0ab36ff0000c5cf7cf#arrow4311765736827995038b1fcc1c624c569fdf206eb351a20bbcbdd4bfae584e0394dafbb6009365aa)" style="fill: none; stroke: black; stroke-width: 2;"></path>

Prod

<path d="M129.515625,120 C129.515625,177.1875 129.515625,177.1875 129.515625,234.375" marker-end="url(http://localhost:3000/builder/5beede0ab36ff0000c5cf7cf#arrow9857085928374139038b1fcc1c624c569fdf206eb351a20bbcbdd4bfae584e0394dafbb6009365aa)" style="fill: none; stroke: black; stroke-width: 2;"></path>

The cubic bezier section C of the path attribute d has identical X coordinates but the Y coordinates are wrong

@jfo84
Copy link
Contributor

jfo84 commented Jan 18, 2019

Long story short @geoff-va, I believe that this is an evil Terser function inlining bug. The code in SvgArrow was getting optimized to pass 0 to most of the arguments in computeStartingAnchorPosition. This is easily the craziest bug I've ever encountered.

Issue in Terser:
terser/terser#244

@jfo84
Copy link
Contributor

jfo84 commented Jan 18, 2019

The workaround is to disable function inlining in UglifyJS or Terser. This works in our production app.

Webpack with Terser example code:

    optimization: {
        ...baseConfig.optimization,
        minimizer: [
            new TerserPlugin({
                cache: true,
                parallel: true,
                terserOptions: {
                    compress: {
                        inline: false,
                    },
                },
            }),
        ],
    },

@geoff-va
Copy link

@jfo84 - Amazing detective work man! This does sound like what I was seeing. I remember seeing C0 instead of like C600 for some of the curve descriptions. I really appreciate you digging into this man! Kudos to you!

@DeveloperAlly
Copy link
Author

@jfo84 I'm using create-react-app, and packaging with docker. I continue to have this issue in prod... not sure how I would go about implementing a workaround here

@jfo84
Copy link
Contributor

jfo84 commented Jan 27, 2019

@AlisonWonderlandApps Are you on v1.0.2? If that doesn't fix it, then you'll have to eject your Webpack config and manually apply the minimizer change above

@DeveloperAlly
Copy link
Author

DeveloperAlly commented Mar 6, 2019

For anyone interested, I used react-app-rewired to override this problem without ejecting my app:

npm install react-app-rewired --save-dev

Then in package.json file change the scripts object to:

"scripts": { "start": "react-app-rewired start", "build": "react-app-rewired build", "test": "react-app-rewired test --env=jsdom", "eject": "react-scripts eject" },

Then create a config.overrides.js file in root directory and put the following in it:

`/* config-overrides.js */
// THIS FILE FIXES THE PRODUCTION ISSUE OF react-archer arrow library.
// PLEASE ALSO LEAVE THE PACKAGE.JSON FILE that referenes rewired in the build script

const OptimizeCSSAssetsPlugin = require('optimize-css-assets-webpack-plugin');
const safePostCssParser = require('postcss-safe-parser');
 
module.exports = function override(config, env) {
  console.log(config.optimization);
  config.optimization.minimizer = [
    new OptimizeCSSAssetsPlugin({
      cssProcessorOptions: {
        parser: safePostCssParser,
        map: {
          // `inline: false` forces the sourcemap to be output into a
          // separate file
          inline: false,
          // `annotation: true` appends the sourceMappingURL to the end of
          // the css file, helping the browser find the sourcemap
          annotation: true,
        },
      },
    }),
  ];
  return config;
};`

@pierpo
Copy link
Owner

pierpo commented May 2, 2019

Thank you so much @jfo84 for investigating successfully this hard problem!
Thank you @AlisonWonderlandApps for proposing a working solution, I'm sure it'll help a lot of people.

Closing this since it's not an issue related to react-archer.

@lishine
Copy link

lishine commented Sep 6, 2019

Next.js 9
This worked for me:

const nextConfig = {
    webpack: (config, { dev }) => {
        config.optimization.minimizer[0].options.terserOptions.compress.inline = false
        return config
    },
}

@gpurgal
Copy link

gpurgal commented Jan 21, 2020

Solution proposed by @AlisonWonderlandApps works but it completely removes TerserPlugin.

@lishine's solution works with react-app-rewired and disables only inlinecompress option in TerserPlugin:

/* config-overrides.js */
module.exports = function override(config, env) {
  config.optimization.minimizer[0].options.terserOptions.compress.inline = false;
  return config;
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants