-
Notifications
You must be signed in to change notification settings - Fork 12k
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
fix(sourcemaps): try to improve the source maps by fixing the path #1028
Conversation
@filipesilva could you review please? |
return builder.bundle('main - [app/**/*]', | ||
const bundles = this.options.bundles || ['app']; | ||
|
||
return builder.bundle('main' + bundles.map(name => `[${name}/**/*]`).join(' - '), |
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.
For bundles = ['app1', 'app2']
this will evaluate to [app1/**/*] - [app2/**/*]
. I think it's missing the first -
.
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.
Have you checked it works as you expect it to btw? It might need to be main - ([app1/**/*] + [app2/**/*])
.
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.
Oh right. Fixed.
I haven't tested it. Doing it right now. |
@@ -242,12 +243,31 @@ class BroccoliTypeScriptCompiler extends Plugin { | |||
* This issue is fixed in https://github.com/Microsoft/TypeScript/pull/5620. | |||
* Once we switch to TypeScript 1.8, we can remove this method. |
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.
Uhm is this comment still accurate? We are on TS 1.8
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.
We still need to fix sources. I need to spend much more time refactoring this plugin and the rest.
Other than verifying that bundling does what you expect and the comment, LGTM. |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This is an attempt to improve the source maps. This works fine with Material.