Skip to content

fix(compiler): Don't strip /*# ... */ comment when css scoping - #16090

Closed
asnowwolf wants to merge 1890 commits into
angular:masterfrom
asnowwolf:dont-strip-hash-comment
Closed

asnowwolf wants to merge 1890 commits into
angular:masterfrom
asnowwolf:dont-strip-hash-comment

Conversation

@asnowwolf

Copy link
Copy Markdown
Contributor

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x")

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

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 style tags properly.

Does this PR introduce a breaking change? (check one with "x")

  • Yes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

@vicb

vicb commented Apr 18, 2017

Copy link
Copy Markdown
Contributor

@asnowwolf what is the difference with #16088 ? could one of them be closed ?

@asnowwolf

asnowwolf commented Apr 19, 2017 •

Copy link
Copy Markdown
Contributor Author

O, this one is not exactly same as #16088, #16088 is only for Chrome, this one is for every browser. In fact, #16088 + #16090 = #14175.

Should I merge them again?

@asnowwolf

asnowwolf commented Apr 20, 2017 •

Copy link
Copy Markdown
Contributor Author

The #16090 's related issue is:
The component style's scss has no source maps. It's hard to locate the source code. long time ago, I submitted a PR to fix that, but later, another commit broken that PR, this PR is a patch for that commit.

The #16088 's related issue is:
Even after fixed the #16090, It's not work properly in Chrome(but Firefox is ok), the root cause is that for the embedded <style> content with sourceMappingURL, Chrome requires an additional sourceURL comment. Maybe it's an issue of Chrome, but we can also resolve it with a little effort.

@vicb vicb added the area: core Issues related to the framework runtime label Apr 26, 2017
@asnowwolf

Copy link
Copy Markdown
Contributor Author

@vicb What else do i need to do? If possible, I hope to merge it as soon as possible because I have to patch the Angular library mannually for each new project. I think maybe there are other people also suffer from these issues.

@OzzieOrca

Copy link
Copy Markdown
Contributor

Could we get this merged? I'm not sure what it is waiting on. It is really nice to have sourcemaps again. (I patched this file locally to test it.)

@daddyschmack

Copy link
Copy Markdown

Hey Guys, Will this be merged in soon?

petebacondarwin and others added 17 commits August 11, 2017 08:09
This commit also simplifies the rules for the aio content and marketing groups
Now that we have upgraded to the latest lunr search engine, the results
from the standard `search` method are more appropriate.
So we do not need to create our own special queries to get good results.
Minor example fix: mark the 'ref' constructor parameter as a private property
- Makes headers consistent
 - Removes extra `)`
 - Makes browserstack line on a separate line
poychang and others added 21 commits August 11, 2017 08:10
…ngular#18532)

Indirectly removes the minimist dependency in the language service
package was added with the addition of `ngc.ts`.
…ngular#18277)

Problem description: when using ngTemplateOutlet with context as
an object literal in a template and binding to the context's property
the embedded view would get re-created even if context object remains
essentially the same (the same shape, just update to one properties).
This happens since currently change detection will re-create object
references when an object literal is used and one of its properties
gets updated through a binding.

Solution: this commit changes ngTemplateOutlet logic so we take
context object shape into account before deciding if we should
re-create view or just update existing context.

Fixes angular#13407
This change allows ReflectiveInjector to be tree shaken resulting
in not needed Reflect polyfil and smaller bundles.

Code savings for HelloWorld using Closure:

Reflective: bundle.js:  105,864(34,190 gzip)
    Static: bundle.js:  154,889(33,555 gzip)
                            645( 2%)

BREAKING CHANGE:

`platformXXXX()` no longer accepts providers which depend on reflection.
Specifically the method signature when from `Provider[]` to
`StaticProvider[]`.

Example:
Before:
```
[
  MyClass,
  {provide: ClassA, useClass: SubClassA}
]

```

After:
```
[
  {provide: MyClass, deps: [Dep1,...]},
  {provide: ClassA, useClass: SubClassA, deps: [Dep1,...]}
]
```

NOTE: This only applies to platform creation and providers for the JIT
compiler. It does not apply to `@Compotent` or `@NgModule` provides
declarations.

Benchpress note: Previously Benchpress also supported reflective
provides, which now require static providers.

DEPRECATION:

- `ReflectiveInjector` is now deprecated as it will be remove. Use
  `Injector.create` as a replacement.

closes angular#18496
Fixes issue uncovered by angular#18388 and re-enables expression
lowering disabled by angular#18513.
The `deploy-to-firebase.sh` always expects there to be a
`src/extra-files/<mode>` directory and breaks if it doesn't exist.
This commit also ensures that if the tests fail, the script exits with an error.
The snippet got lost some time during the migration from the old version (it is
[present in v2][1]).

[1]: https://v2.angular.io/docs/ts/latest/cookbook/aot-compiler.html#!#running-the-application

Fixes angular#18544
)

This commit also updates the version of `@angular/cli` used for docs examples.
The previous (transient) dependency `@ngtools/webpack` was not compatible with
`@angular/compiler-cli@>=5` and was breaking when running against the local
builds (currently at 5.0.0-beta.2). The version of `@ngtools/webpack` used by
the latest `@angular/cli` version is compatible with `@angular/compiler-cli@5`.
…ar#18536)

This commit adds support for setting default `updateOn` values
in `FormGroups` and `FormArrays`. If you set `updateOn` to
’blur’` at the group level, all child controls will default to `’blur’`,
unless the child has explicitly specified a different `updateOn` value.

```
const c = new FormGroup({
   one: new FormControl()
}, {updateOn: blur});
```

 It's worth noting that parent groups will always update their value and
validity immediately upon value/validity updates from children. In other
words, if a group is set to update on blur and its children are individually
set to update on change, the group will still update on change with its
children; its default value will simply not be used.
@asnowwolf
asnowwolf force-pushed the dont-strip-hash-comment branch from c460df5 to 99d6140 Compare August 11, 2017 00:15
@googlebot

Copy link
Copy Markdown

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@asnowwolf

Copy link
Copy Markdown
Contributor Author

Sorry, when i merge the latest branch, a misuse destroys this PR.

#18641 is the re-pushed version that has merged the latest master.

@angular-automatic-lock-bot

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area: core Issues related to the framework runtime cla: no type: bug/fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.