-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
Ignore missing sourcemap if referenced file does not exist #55
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Only one minor thing around using console unguarded (therefore not honoring the silent flag from ember-cli), but that could totally be addressed in a follow up...
index.js
Outdated
if (fs.existsSync(sourceMapPath)) { | ||
sourceMap.content = JSON.parse(fs.readFileSync(sourceMapPath)); | ||
} else { | ||
console.warn(`[WARN] (broccoli-uglify-sourcemap) "${url}" referenced in "${relativePath}" could not be found`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you modify this to use this.console.warn
(then initialize this.console = options.console || console
in the constructor)? There is at least one other usage of console.warn
also.
The idea is to have ember-cli-uglify pass in a “custom console” which uses our UI
object and avoiding needing to manually check for --silent
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I noticed that too, but wanted to address it in a separate PR. your point about honoring --silent
is valid though, this should probably be something like } else if (!this.silent) {
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rwjblue adjusted
5987c38
to
dbb6616
Compare
@@ -45,6 +46,9 @@ function third(){ | |||
"something.css": "#id { | |||
background: white; | |||
}", | |||
"with-broken-sourcemap.js": "function meaningOfLife(){throw new Error(42)} | |||
//# sourceMappingURL=with-broken-sourcemap.map", | |||
"with-broken-sourcemap.map": "{\\"version\\":3,\\"sources\\":[\\"0\\"],\\"names\\":[\\"meaningOfLife\\",\\"Error\\"],\\"mappings\\":\\"AAAA,SAASA,gBAEP,MAAM,IAAIC,MADa\\",\\"file\\":\\"with-broken-sourcemap.js\\"}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way we can output "sources: ["with-broken-sourcemap.js]
instead of sources: ["0"]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know. can you explain why that would be useful or more correct than what we currently have?
Resolves #53