Bundle un-minified assets #184
Conversation
The ember build will handle minification and mangling in production depending on the apps configuration.
|
wow ~4kb gzipped is a lot! That said I can also concur that I established some time ago that letting ember build do the minification produces smaller vendor bundles. I like this PR. |
|
LGTM, thanks! @jrjohnson any interest in taking of this addon? It's not something I'm currently using but is still widely used - so I'd hate to archive it if others find it valuable. |
|
Versioned in 3.8.0 |
|
@jasonmit after seeing some stuff from the moment maintainers warning everyone against using moment I'm going to try and move our codebase away from it, but that will probably take a while. That is to say I'd be happy to help out here but would like to make a concerted push towards deprecating this addon and relying on embroider and auto-import to replace it. |
|
I haven't used moment in a couple years, totally get the push to migrate off.
Yeah, that's the work that is needed that hasn't happened - #164 I'll add you as a maintainer now. |
|
@jrjohnson No idea how... But this PR makes not ok 1 Chrome 84.0 - [undefined ms] - Global error: Uncaught TypeError: Cannot read property 'version' of undefined at https://localhost:7357/assets/vendor.js, line 10618
---
browser log: |
testContext: [object Object]
ERROR: Uncaught TypeError: Cannot read property 'version' of undefined at https://localhost:7357/assets/vendor.js, line 10618
...
not ok 2 Chrome 84.0 - [undefined ms] - Global error: Uncaught Error: Could not find module @ember/-internals/runtime required by: ember-testing/lib/test/promise at https://localhost:7357/assets/test-support.js, line 23
---
browser log: |
testContext: [object Object]
ERROR: Uncaught Error: Could not find module @ember/-internals/runtime required by: ember-testing/lib/test/promise at https://localhost:7357/assets/test-support.js, line 23
...
not ok 3 Chrome 84.0 - [undefined ms] - Global error: Uncaught Error: Could not find module `ember-resolver` imported from `camilyo/resolver` at https://localhost:7357/assets/vendor.js, line 252
---
browser log: |
testContext: [object Object]
ERROR: Uncaught Error: Could not find module `ember-resolver` imported from `camilyo/resolver` at https://localhost:7357/assets/vendor.js, line 252
...
not ok 4 Chrome 84.0 - [undefined ms] - Global error: Uncaught Error: Could not find module `@ember/test-helpers` imported from `camilyo/tests/test-helper` at https://localhost:7357/assets/vendor.js, line 252
---
browser log: |
testContext: [object Object]
ERROR: Uncaught Error: Could not find module `@ember/test-helpers` imported from `camilyo/tests/test-helper` at https://localhost:7357/assets/vendor.js, line 252
...
not ok 5 Chrome 84.0 - [undefined ms] - Global error: Uncaught TypeError: Ember.assert is not a function at https://localhost:7357/905622077799/tests/index.html?hidepassed, line 38
---
browser log: |
testContext: [object Object]
ERROR: Uncaught TypeError: Ember.assert is not a function at https://localhost:7357/905622077799/tests/index.html?hidepassed, line 38
...
not ok 6 [undefined ms] - Error
---
message: >
Received SIGINT signal
...
1..6
# tests 6
# pass 0
# skip 0
# fail 6
Received SIGINT signal |
| development: 'vendor/moment/min/moment-with-locales.js', | ||
| production: 'vendor/moment/min/moment-with-locales.min.js' | ||
| }, | ||
| 'vendor/moment/moment-with-locales.js', |
There was a problem hiding this comment.
This should be 'vendor/moment/min/moment-with-locales.js' I think
There was a problem hiding this comment.
It's been a while, but IIRC including those minified was exactly what was breaking before. Maybe it's fixed upstream now, I have no idea. Happy to take a PR as long as it's tested with the latest versions of ember-cli and ember-cli-terser.
There was a problem hiding this comment.
not sure if it's minified or not, but the path is wrong, there's nothing at the path current specified in 3.8!
There was a problem hiding this comment.
These paths have changed within minor versions of moment in the past. Figured I'd call that out for anyone looking at a solution.
There was a problem hiding this comment.
it's not a problem that the path changed, it's a problem that the build process is broken because it references the wrong path
There was a problem hiding this comment.
I see - @mehulkar feel free to open a PR once you verify.
Broken import path was breaking ember server. The issue was the path referenced was broken. As per this recommendation, jasonmit#184
The ember build will handle minification and mangling in production
depending on the apps configuration so it doesn't need to be done here.
Interestingly this results in ~4kb less JS (gzipped) in vendor for my app. I guess the double minification causes bloat.
Fixes #183