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

Running ng build with optimization set to true breaks this library #442

Closed
i3anaan opened this issue Jul 30, 2019 · 7 comments
Closed

Running ng build with optimization set to true breaks this library #442

i3anaan opened this issue Jul 30, 2019 · 7 comments
Labels

Comments

@i3anaan
Copy link

i3anaan commented Jul 30, 2019

Problem

Apperantly setting "optimzation": true in angular.json causes an app that functions normally without optimization (i.e. debug/development) to break with optimization (production mode).

Since Angular's stance on the matter is pretty much that the library should fix it, I'm posting an issue for it here.

Steps to reproduce:

  • Have an angular project using skyhook
  • build using "optimzation": true (i.e. ng build --prod)
  • Try and use skyhook by dragging an object
  • Experience it not working

Error in console:

Uncaught Invariant Violation: Cannot call endDrag while not dragging.
    at QLaP.e.exports (https://url/main.2884ec44a6f193632cc5.js:1:238563)
    at https://url/main.2884ec44a6f193632cc5.js:1:1088445
    at uF.<anonymous> (https://url/main.2884ec44a6f193632cc5.js:1:1088504)
    at Object.i.<computed> [as beginDrag] (https://url/main.2884ec44a6f193632cc5.js:1:1088678)
    at handleTopDragStart (https://url/main.2884ec44a6f193632cc5.js:1:1255778)
    at t.invokeTask (https://url/polyfills.c67528456ba5ac04556c.js:1:8753)
    at Object.onInvokeTask (https://url/polyfills.c67528456ba5ac04556c.js:1:5730)
    at t.invokeTask (https://url/polyfills.c67528456ba5ac04556c.js:1:8674)
    at e.runTask (https://url/polyfills.c67528456ba5ac04556c.js:1:3935)
    at e.invokeTask [as invoke] (https://url/polyfills.c67528456ba5ac04556c.js:1:9836)

Versions

Angular cli 7.2.2
Skyhook 1.2.1

Expected result

App works the same with optimization enabled or disabled.

Workaround

Disable optimization by setting "optimization": false in angular.json

Request

Make this library work with optimization set to true.

@cormacrelf
Copy link
Owner

cormacrelf commented Jul 30, 2019

This library doesn't have any side-effecting getters. It has a bunch of getters, but they are pure. At least, I have ripgrepped the codebase pretty hard and examined the ones it has. If you can find one yourself, let me know.

If it's a getter problem, it's either upstream (dnd-core) or in your code. My guess from your stack trace is the latter (it has beginDrag a few frames back). Edit: being in your code is supported by the fact that ng build --prod is working just fine on the examples page.

It's not necessarily a getter problem, though. I'll think about what else it could be.

@i3anaan
Copy link
Author

i3anaan commented Jul 30, 2019

Hey, thank you for the quick response!

I checked our code, and there is not a single place where we call endDrag ourselves, thus making me think it cannot be our fault.
If this assumption is wrong and it could still be our fault please say so, then I will do another more thorough examination.
Edit: I checked our code quickly, none of our getters seem to have side-effects.

@i3anaan
Copy link
Author

i3anaan commented Jul 30, 2019

I also did a quick search in dnd-core, and found this issue that is atleast somewhat similar: react-dnd/react-dnd#1049

Will have another, better, look tomorrow.

@i3anaan
Copy link
Author

i3anaan commented Jul 31, 2019

So this morning I did some searching around and I found more issues on dnd-core related to minimization. So if you agree that they should fix this we can close this issue here, and I'll make a new one over there.

Some related issues on dnd-core:
https://github.com/react-dnd/react-dnd/issues?utf8=%E2%9C%93&q=is%3Aissue+minimize
None of them over any real solution other than to disable the minification.

@i3anaan
Copy link
Author

i3anaan commented Jul 31, 2019

Wew, after what looked like it was turning into a very big headache this morning turned out pretty great, I found a workaround!

Basically it boils down to not using UglifyJS as minimizer in webpack, as that somehow breaks something somewhere related to using drag and drop (in skyhook). Instead I switched to using Terser, which is actually planned to be the default in webpack 5. To achieve this I made a sort of hack for the @angular-devkit/build-angular package using the patch-package package.

patch-package was used to generate a patch for the @angular-devkit/build-angular package (I used this site as a how-to). I attached the patch file's content at the bottom. Then I set up a prepare hook in the package.json like this:

  "scripts": {
    "prepare": "patch-package",
    ...
    }

And it works! :D


Patch file used:

diff --git a/node_modules/@angular-devkit/build-angular/src/angular-cli-files/models/webpack-configs/common.js b/node_modules/@angular-devkit/build-angular/src/angular-cli-files/models/webpack-configs/common.js
index 99903a9..717f3bb 100644
--- a/node_modules/@angular-devkit/build-angular/src/angular-cli-files/models/webpack-configs/common.js
+++ b/node_modules/@angular-devkit/build-angular/src/angular-cli-files/models/webpack-configs/common.js
@@ -22,7 +22,7 @@ const find_up_1 = require("../../utilities/find-up");
 const utils_2 = require("./utils");
 const ProgressPlugin = require('webpack/lib/ProgressPlugin');
 const CircularDependencyPlugin = require('circular-dependency-plugin');
-const UglifyJSPlugin = require('uglifyjs-webpack-plugin');
+const TerserPlugin = require('terser-webpack-plugin');
 const StatsPlugin = require('stats-webpack-plugin');
 /**
  * Enumerate loaders and their dependencies from this file to let the dependency validator
@@ -267,12 +267,7 @@ function getCommonConfig(wco) {
                     // component styles retain their original file name
                     test: (file) => /\.(?:css|scss|sass|less|styl)$/.test(file),
                 }),
-                new UglifyJSPlugin({
-                    sourceMap: buildOptions.sourceMap,
-                    parallel: true,
-                    cache: true,
-                    uglifyOptions,
-                }),
+                new TerserPlugin(),
             ],
         },
         plugins: extraPlugins,

@cormacrelf
Copy link
Owner

You should probably run your (unfixed) code in a debugger with pausing on exceptions enabled (and source maps). Then you’ll know where it actually is.

@cormacrelf
Copy link
Owner

Closing because the examples still work with --prod so it’s not this library’s problem.

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

No branches or pull requests

2 participants