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

Runtime error after build with --optimize #64

Open
atlewee opened this issue Jun 9, 2023 · 10 comments
Open

Runtime error after build with --optimize #64

atlewee opened this issue Jun 9, 2023 · 10 comments

Comments

@atlewee
Copy link

atlewee commented Jun 9, 2023

First of all, Amazing tool !
Works perfect for me in development, but tried to build a production version just now and got this error:
image
( Seems to be a elm-ui text input )

I am using the postprocess.js file from the example here with no modifications.
Any idea what can cause this?

Also, the editor shows an error here. (But no errors when building)
image

dependency versions:
image

@lydell
Copy link
Owner

lydell commented Jun 9, 2023

Hi!

  1. That default_options is underlined with red in the editor is because it is missing in uglify-js type definitions for some reason. I kept the example simple, but in my own code I have a TypeScript disable comment there.
  2. Does the error happen if you don’t use uglify-js? In other words, if you temporarily remove your postprocess does it still happen? Prediction/hope: Yes
  3. Does the error happen when running elm-watch hot and using the browser UI to switch to optimize mode? Or does it happen when you have run elm-watch make --optimize? Predicion/hope: Only the former, because in hot mode I do some very tricky replacements on the compiled code for hot reloading, and I’ve had an edge case bug there before. But for the latter, elm-watch acts just as a wrapper around elm make with no modifications so in that case there’s not much room for bugs.

Another guess is that this is an uglify-js bug, or a bug in my example configuration of uglify-js.

@atlewee
Copy link
Author

atlewee commented Jun 9, 2023

Thank you for fast response!

the issue goes away if I remove this line from my elm-watch.json:

"postprocess": ["elm-watch-node", "postprocess.js"],

So seems related to uglify or the postprocess.js file
elm-watch hot works fine, also when changing between debug/standard/optimized
(everything works as long as that postprocess step is removed) :)

@lue-bird
Copy link

lue-bird commented Jun 9, 2023

Yeah I've had encountered something similar – strangely always and only when more than one elm-ui element inside a centered column/row is centered.

this is (an uglify-js bug, or) a bug in my example configuration of uglify-js

I'd say this is highly likely, especially because configurations with for example only eol2 don't break the app in these situations.

@atlewee
Copy link
Author

atlewee commented Jun 9, 2023

aha.. The error goes away if i replace my Input.text element with (text "test")
Strange...
( I have no explicit centering going on any parents of that text input )

@lydell
Copy link
Owner

lydell commented Jun 9, 2023

It would be amazing if you could reduce your code into a small example that reproduces the issue. Then we could dig deeper into uglify-js:

  • Maybe there’s some option to it that we can toggle.
  • Maybe there’s a bug we can report, or find already reported.

@atlewee
Copy link
Author

atlewee commented Jun 10, 2023

Here is a small example that crashes:

Main.elm

module Main exposing (main)

import Element
import Element.Input as Input


type Msg
    = NoOp


main =
    Element.layout [] -- Converts from Element to Html
        (Input.text
            []
            { label = Input.labelHidden ""
            , onChange = always NoOp
            , placeholder = Nothing
            , text = ""
            }
        )

Other files for reference:

Index.html

<!DOCTYPE html>
<html lang="en">

<head>
    <meta charset="UTF-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>example-minimal</title>
</head>

<body>
    <div id="root"></div>
    <script src="./build/main.js"></script>
    <script>
        Elm.Main.init({ node: document.getElementById("root") });
    </script>
</body>

</html>

package.json

{
  "type": "module",
  "devDependencies": {
    "esbuild": "^0.18.0",
    "uglify-js": "^3.17.4"
  }
}

elm-watch.json

{
  "postprocess": ["elm-watch-node", "postprocess.js"],
  "targets": {
    "My target name": {
      "inputs": ["src/Main.elm"],
      "output": "build/main.js"
    }
  }
}

elm.json

{
    "type": "application",
    "source-directories": [
        "src"
    ],
    "elm-version": "0.19.1",
    "dependencies": {
        "direct": {
            "elm/browser": "1.0.2",
            "elm/core": "1.0.5",
            "elm/html": "1.0.0",
            "mdgriffith/elm-ui": "1.1.8"
        },
        "indirect": {
            "elm/json": "1.1.3",
            "elm/time": "1.0.0",
            "elm/url": "1.0.0",
            "elm/virtual-dom": "1.0.3"
        }
    },
    "test-dependencies": {
        "direct": {},
        "indirect": {}
    }
}

And the postprocess.js file is alos copied from this repo/example

@lydell
Copy link
Owner

lydell commented Jun 10, 2023

Small update so far: By trial and error I have found that if I comment out collapse_vars: true, the runtime error goes away.

@lydell
Copy link
Owner

lydell commented Jun 10, 2023

Other notes:

  • I have removed elm-watch from the picture. elm make --optimize + uglify is enough to reproduce.
  • I removed the esbuild minification to retain long names of things to make debugging easier.
  • I format the output with Prettier to make debugging easier.
  • Diffing with and without collapse_vars: true:
     $mdgriffith$elm_ui$Element$Input$hasFocusStyle = function (attr) {
-      return (
-        4 === attr.$ &&
-        11 === attr.b.$ &&
-        !attr.b.a &&
-        ((attr = attr.b), attr.a, true)
-      );
+      return attr.b.a, 4 === attr.$ && 11 === attr.b.$ && !attr.b.a && true;
     },

That transformation is wrong. Currently trying to reduce the compiled JS to make a small example that can be reported to uglify (or searched for in their issue tracker).

@lydell
Copy link
Owner

lydell commented Jun 11, 2023

New finding: Adding side_effects: true, also makes the runtime error go away. (I haven’t measured how it affects speed and size yet.)

@lydell
Copy link
Owner

lydell commented Jun 11, 2023

I managed to make a bug report for uglify: mishoo/UglifyJS#5794

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

3 participants