uglify-js is unconditionally imported, but only listed as optional dependency
#1391
Comments
|
This would need to be a 4.x fix right? |
|
either that or we update the dependency in https://github.com/ember-cli/broccoli-middleware/blob/master/package.json#L22. not sure if that's possible though 🤔 |
|
I can do the release if anyone creates a PR. And yes, it should be a PR for the 4.x branch. I'll merge into master afterwards. What about 3.x? |
nknapp
added a commit
that referenced
this issue
Oct 13, 2017
closes #1391 uglify-js is an optional dependency and should be treated as such. This commit gracefully handle MODULE_NOT_FOUND errors while loading uglify.
nknapp
added a commit
that referenced
this issue
Oct 13, 2017
closes #1391 uglify-js is an optional dependency and should be treated as such. This commit gracefully handles MODULE_NOT_FOUND errors while loading uglify.
nknapp
added a commit
that referenced
this issue
Oct 13, 2017
closes #1391 uglify-js is an optional dependency and should be treated as such. This commit gracefully handles MODULE_NOT_FOUND errors while loading uglify.
nknapp
added a commit
that referenced
this issue
Oct 13, 2017
closes #1391 uglify-js is an optional dependency and should be treated as such. This commit gracefully handles MODULE_NOT_FOUND errors while loading uglify.
nknapp
added a commit
that referenced
this issue
Oct 13, 2017
closes #1391 uglify-js is an optional dependency and should be treated as such. This commit gracefully handles MODULE_NOT_FOUND errors while loading uglify.
nknapp
added a commit
that referenced
this issue
Oct 17, 2017
closes #1391 uglify-js is an optional dependency and should be treated as such. This commit gracefully handles MODULE_NOT_FOUND errors while loading uglify. - Check for existing uglify-js (and load uglify-js) only if minification was activated - Use "require.resolve" to check if uglify exists. Otherwise, a missing dependency of uglify-js would cause the same behavior as missing uglify-js. (Only a warning, no error) - The code to load and run uglify is put into a single for readability purposes - Tests use a mockup Module._resolveFilename to simulate the missing module. This function is used by both "require" and "require.resolve", so both are mocked equally.
nknapp
added a commit
that referenced
this issue
Oct 17, 2017
closes #1391 uglify-js is an optional dependency and should be treated as such. This commit gracefully handles MODULE_NOT_FOUND errors while loading uglify. - Check for existing uglify-js (and load uglify-js) only if minification was activated - Use "require.resolve" to check if uglify exists. Otherwise, a missing dependency of uglify-js would cause the same behavior as missing uglify-js. (Only a warning, no error) - The code to load and run uglify is put into a single for readability purposes - Tests use a mockup Module._resolveFilename to simulate the missing module. This function is used by both "require" and "require.resolve", so both are mocked equally.
nknapp
added a commit
that referenced
this issue
Oct 17, 2017
closes #1391 uglify-js is an optional dependency and should be treated as such. This commit gracefully handles MODULE_NOT_FOUND errors while loading uglify. - Check for existing uglify-js (and load uglify-js) only if minification was activated - Use "require.resolve" to check if uglify exists. Otherwise, a missing dependency of uglify-js would cause the same behavior as missing uglify-js. (Only a warning, no error) - The code to load and run uglify is put into a single for readability purposes - Tests use a mockup Module._resolveFilename to simulate the missing module. This function is used by both "require" and "require.resolve", so both are mocked equally. (cherry picked from commit d5caa56)
|
Released in 4.0.11 |
|
awesome, thanks :) |
I think our issue was only with 4.x. Since 5.x seems to be the current release I think we're probably fine without another 3.x release. |
|
The current release is 4.x. There has not been any 5.x release yet |
|
🤔 thought I had seen 5.x somewhere... nevermind, sorry about the noise :D |
|
I bumped the master branch to 5.0-alpha. But there is no release at the moment and I'm only doing bugfixes |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This is causing ember-cli/ember-cli#7371, where people are using
npmto installhandlebarsas a transitive dependency but apparentlynpmis not installing the optionaluglify-jsdependency (anymore?) which makes the import fail.The text was updated successfully, but these errors were encountered: