Repository navigation
fix(compiler): Don't strip /*# ... */ comment when css scoping - #18641
Conversation
|
Sorry, when i merge the latest branch, a misuse destroys #16090 , this one is a re-pushed version that has merged the latest master. |
There was a problem hiding this comment.
could we make it more robust by using a replace fn that would:
- check if
[0] === '#' - check if the content contains
sourceURL=orsourceMappingURL=
There was a problem hiding this comment.
Did you mean that all of the comments that begin with # are likely to leave something that we do not want to keep, So we should only ignore the known sourceURL and sourceMappingURL? If so, I will try to revise it.
fded4c3 to
ef337b0
Compare
|
There are some questions: whether we should support the legacy format |
|
It seems that CI has failed, but I have been tested in the local development environment, and CI's failure seems to be due to random factors. Do I need to trigger CI until it runs successfully? |
|
Looks due to a flake to me, i'd give suggest giving it a rerun |
ef337b0 to
26291f4
Compare
|
The |
There was a problem hiding this comment.
I think you could just change the RegExp to /\/\*(?!# source(?:Mapping)?URL=)[\s\S]+?\*\//g and leave everything else as it was.
(Not that it matters much 😁)
There was a problem hiding this comment.
Yes, I think it's better too.
f36c4a4 to
0bcc85a
Compare
|
The changes LGTM. We should merge to master only as the behavior is different (we now keep comment when they contain source maps). @asnowwolf could you please change the commit message to match the changes you've made and give more info: Also please add links to the issues related to this PR ( |
0bcc85a to
94e1b4c
Compare
|
Done, but here is not a related issue, the relevant issue in the angular-cli repository. |
94e1b4c to
62d796b
Compare
Fix CSS source mapping for component by keeping `/*# sourceMappingURL= ... */` and `/*# sourceURL= ... */` comments. Relates to <angular/angular-cli#4199>.
|
Changes LGTM. |
|
BTW: What is going to do with #16088 ? It is aimed at a BUG that appears only in Chrome (and possibly a feature in Chrome because it's been there for a long time, so there's a special Chrome plugin to fix it). In addition, it can be modified by modifying the Angular-CLI or other toolchain. Is it worth merging here? Do I need to turn it off and to find other ways to solve it? |
|
Can this be fixed in a 4.x release as well? Behavior changed but how would it affect anything? Also it's mentioned in the changelog as a bugfix, not a breaking change. |
|
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. |
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x")
What is the current behavior? (You can also link to an open issue here)
Currently, the source-mapping comments of component style is stripped by extractStyleUrls, so, css mappings don't work properly for component styles.
What is the new behavior?
Don't strip hash comments, so that they will be past to
styletags properly.Does this PR introduce a breaking change? (check one with "x")
If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...
Other information: