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

Minification leads to broken application #2116

Closed
domoritz opened this issue Oct 7, 2018 · 22 comments
Closed

Minification leads to broken application #2116

domoritz opened this issue Oct 7, 2018 · 22 comments
Labels
🐛 Bug Stale Inactive issues

Comments

@domoritz
Copy link

domoritz commented Oct 7, 2018

🐛 bug report

Forked off of #1887 with a specific issue described in vega/falcon#94.

🤔 Expected Behavior

Minification should not break the page.

😯 Current Behavior

In the minified version of the MapD demo, drawing brushes does not work.

💻 Code Sample

See https://github.com/uwdata/falcon/

🌍 Your Environment

Software Version(s)
Parcel 1.10.1
Node v10.11.0
@kzc
Copy link

kzc commented Oct 17, 2018

This framework depends on creating things based on their original class and function names. Create a .terserrc file and enable keep_classnames and keep_fnames to prevent them from being mangled.

@domoritz
Copy link
Author

@kzc Thank you but now I get this error:

/logo/favicon.png: JSON5: invalid character 'k' at 1:1

I have no idea what the json module is trying to do with the favicon.

@kzc
Copy link

kzc commented Oct 17, 2018

Looks like an unrelated Parcel issue with non JS assets. Post the entire error and stack trace.

If you remove the favicon does it build and work?

@domoritz
Copy link
Author

Unfortunately, no.

flights/index.ts: JSON5: invalid character 'k' at 1:1

@kzc
Copy link

kzc commented Oct 17, 2018

No stack trace?

@domoritz
Copy link
Author

Sorry, I forgot to post it. Yes, there is a stack trace:

yarn parcel build --public-url '/falcon/' flights/index.html
yarn run v1.10.1
$ /Users/domoritz/Developer/UW/crossfilter/node_modules/.bin/parcel build --public-url /falcon/ flights/index.html
🚨  /Users/domoritz/Developer/UW/crossfilter/flights/index.ts: JSON5: invalid character 'k' at 1:1
    at syntaxError (/Users/domoritz/Developer/UW/crossfilter/node_modules/parcel-bundler/node_modules/json5/lib/parse.js:1:12963)
    at invalidChar (/Users/domoritz/Developer/UW/crossfilter/node_modules/parcel-bundler/node_modules/json5/lib/parse.js:1:12177)
    at Object.value (/Users/domoritz/Developer/UW/crossfilter/node_modules/parcel-bundler/node_modules/json5/lib/parse.js:1:3762)
    at lex (/Users/domoritz/Developer/UW/crossfilter/node_modules/parcel-bundler/node_modules/json5/lib/parse.js:1:1680)
    at parse (/Users/domoritz/Developer/UW/crossfilter/node_modules/parcel-bundler/node_modules/json5/lib/parse.js:1:992)
    at Object.load (/Users/domoritz/Developer/UW/crossfilter/node_modules/parcel-bundler/src/utils/config.js:45:30)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

@domoritz
Copy link
Author

domoritz commented Oct 17, 2018

Here is the stack trace with favicons

yarn build:demos
yarn run v1.10.1
$ parcel build --public-url '/falcon/' flights/index.html flights-mapd/index.html weather/index.html --detailed-report
🚨  /Users/domoritz/Developer/UW/crossfilter/logo/favicon.png: JSON5: invalid character 'k' at 1:1
    at syntaxError (/Users/domoritz/Developer/UW/crossfilter/node_modules/parcel-bundler/node_modules/json5/lib/parse.js:1:12963)
    at invalidChar (/Users/domoritz/Developer/UW/crossfilter/node_modules/parcel-bundler/node_modules/json5/lib/parse.js:1:12177)
    at Object.value (/Users/domoritz/Developer/UW/crossfilter/node_modules/parcel-bundler/node_modules/json5/lib/parse.js:1:3762)
    at lex (/Users/domoritz/Developer/UW/crossfilter/node_modules/parcel-bundler/node_modules/json5/lib/parse.js:1:1680)
    at parse (/Users/domoritz/Developer/UW/crossfilter/node_modules/parcel-bundler/node_modules/json5/lib/parse.js:1:992)
    at Object.load (/Users/domoritz/Developer/UW/crossfilter/node_modules/parcel-bundler/src/utils/config.js:45:30)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

@DeMoorJasper
Copy link
Member

@domoritz seems like one of your config files is invalid JSON, probably the tsconfig

@domoritz
Copy link
Author

@DeMoorJasper The errors go away when I don't use a .terserrc file.

Here is my tsconfig file

{
  "compilerOptions": {
    "module": "esnext",
    "target": "es2017",
    "outDir": "build",
    "moduleResolution": "node",
    "esModuleInterop": true,
    "noImplicitAny": false,
    "strictNullChecks": true,
    "noImplicitReturns": true,
    "noImplicitThis": true,
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "sourceMap": true,
    "allowSyntheticDefaultImports": true,
    "skipLibCheck": true,
    "resolveJsonModule": true
  },
  "files": ["src/index.ts", "flights/index.ts", "weather/index.ts"]
}

@DeMoorJasper
Copy link
Member

DeMoorJasper commented Oct 17, 2018

@domoritz do you have any other config files, that one is valid. Ow just read about your terserrc fixing it, than that one is invalid JSON

@domoritz
Copy link
Author

🤦‍♂️ I thought a .terserrc file is plain text and not JSON! It works now.

@domoritz
Copy link
Author

Unfortunately, I still does not work. See https://uwdata.github.io/falcon/flights-mapd/. You can check the full code at https://github.com/uwdata/falcon.

@kzc
Copy link

kzc commented Oct 17, 2018

Please post your .terserrc file. You may have to blow away the Parcel cache and rebuild everything from scratch in the event it's not aware of this file.

https://uwdata.github.io/falcon/flights-mapd/ seems to function and is interactive with all graphs updating in FF at least. Without knowing anything about your app or its terminology - what exactly is wrong with it?

It would be easier to troubleshoot if you produced the smallest possible program exhibiting an error.

@domoritz
Copy link
Author

domoritz commented Oct 17, 2018

The charts all load but when you start brushing in one view, the other charts should show histograms. Instead, I only see the grey bars of the background. The overall count chart updates fine.

Bad

screen shot 2018-10-17 at 17 57 23

Good

screen shot 2018-10-17 at 17 57 34

Here is the .terserrc file: https://github.com/uwdata/falcon/blob/master/.terserrc

I trashed the cache and dist folder as you can see in https://github.com/uwdata/falcon/blob/master/package.json#L14.

I wish I had a smaller example. I am a maintainer of a popular visualization library and I understand that I would make it easier for you if I had a complete but small example. I really appreciate your help here and try my best to thoroughly explain the issue I see and steps I have taken.

@kzc
Copy link

kzc commented Oct 17, 2018

Strange. When I made my last comment I was seeing the "Good" state, but now that I've reloaded your latest version it is showing the "Bad" state.

You could try to narrow down what the minify issue is by alternatively using "mangle": false and "compress": false in .terserrc, as well as in combination. If compress is the culprit you can start disabling individual compress options one by one. Isolating the problem in an application of this size won't be easy.

@domoritz
Copy link
Author

domoritz commented Oct 17, 2018

Probably some caching issue.

I'll get back to you when I could try these options. Thank you for suggesting a way to debug this.

@domoritz
Copy link
Author

Ahh, the problem is unused.

With

{
  "compress": {
    "unused": false
  }
}

It works.

@domoritz
Copy link
Author

"unused": "keep_assign" is sufficent.

@kzc
Copy link

kzc commented Oct 17, 2018

"unused": "keep_assign" is sufficent.

Good to see you've found a workaround. If you ever make a reduced Terser bug repro that would be helpful.

unused failing is usually a symptom of another high level compress bug. Try each of these compress settings individually with "unused": true (its default setting):

  • "collapse_vars": false
  • "reduce_vars": false
  • "inline": 2
  • "inline": 1
  • "inline": 0
  • "toplevel": false

Less likely:

  • "comparisons": false
  • "typeofs": false

For bonus points also make note of the minified bundle size for each option.

@domoritz
Copy link
Author

Unfortunately, I'm super busy with school stuff right now but if anyone wants to try, https://github.com/uwdata/falcon is open source. I might get to this later next week.

@github-actions github-actions bot added the Stale Inactive issues label Jan 17, 2020
@domoritz
Copy link
Author

I manually resolved this issue by disabling optimizations: vega/falcon@503c8b7. I don't think it's a complete fix, though.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs.

@github-actions github-actions bot added the Stale Inactive issues label Jul 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug Stale Inactive issues
Projects
None yet
Development

No branches or pull requests

3 participants