Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Comments

Close side panel

fix(compiler): apply style on :host attribudes in prod builds.#49118

Closed
JeanMeche wants to merge 1 commit intoangular:mainangular/angular:mainfrom
JeanMeche:fix/48558JeanMeche/angular:fix/48558Copy head branch name to clipboard
Closed

fix(compiler): apply style on :host attribudes in prod builds.#49118
JeanMeche wants to merge 1 commit intoangular:mainangular/angular:mainfrom
JeanMeche:fix/48558JeanMeche/angular:fix/48558Copy head branch name to clipboard

Conversation

@JeanMeche
Copy link
Member

@JeanMeche JeanMeche commented Feb 16, 2023 •
edited
Loading

In prod builds, selectors are optimized and spaces a removed. #48558 introduced a regression on selectors without spaces. This commit fixes this.

Fixes #49100

Currently this PR blocks #49169.

PR Type

What kind of change does this PR introduce?

  • Bugfix

Does this PR introduce a breaking change?

  • No

@pullapprove pullapprove bot requested a review from alxhub February 16, 2023 23:29
This was referenced Feb 16, 2023
Closed
Closed
@JeanMeche
Copy link
Member Author

JeanMeche commented Feb 25, 2023

I wouldn't expect this commit to trigger regressions. Are the failling internal tests a false positive ?

@dylhunn
Copy link
Contributor

dylhunn commented Feb 28, 2023 •
edited
Loading

The presubmit failures are numerous but apparently unrelated. This probably got bundled for presubmit with a bad pending change. Let's run it again.

edit: presubmit running

@dylhunn dylhunn added the action: presubmit The PR is in need of a google3 presubmit label Feb 28, 2023
@jessicajaniuk jessicajaniuk added the area: compiler Issues related to `ngc`, Angular's template compiler label Mar 2, 2023
@ngbot ngbot bot added this to the Backlog milestone Mar 2, 2023
@JeanMeche
Copy link
Member Author

JeanMeche commented May 1, 2023

@dylhunn How did the presubmit go ?

@JeanMeche JeanMeche mentioned this pull request May 9, 2023
Closed
@JeanMeche JeanMeche force-pushed the fix/48558 branch 2 times, most recently from 5425ea6 to ecbaee1 Compare June 5, 2023 20:40
@alan-agius4 alan-agius4 added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Jun 9, 2023
@alan-agius4
Copy link
Contributor

alan-agius4 commented Jun 9, 2023

Presubmit

@dylhunn
Copy link
Contributor

dylhunn commented Jun 9, 2023

Just checked Alan's new presubmit. It's clean, modulo 12 flakes and preexisting failures. So we should be good to go in that regard.

@dylhunn dylhunn requested a review from AndrewKushnir June 9, 2023 23:58
JoostK
JoostK reviewed Jun 10, 2023
View reviewed changes
packages/compiler/src/shadow_css.ts Outdated Show resolved Hide resolved
packages/compiler/src/shadow_css.ts Outdated Show resolved Hide resolved
@JeanMeche JeanMeche force-pushed the fix/48558 branch 2 times, most recently from 7236b4b to a3cac72 Compare June 10, 2023 20:41
@JoostK JoostK mentioned this pull request Jun 12, 2023
Closed
@AndrewKushnir AndrewKushnir requested a review from JoostK June 20, 2023 03:54
JoostK
JoostK approved these changes Jun 20, 2023
View reviewed changes
Copy link
Member

@JoostK JoostK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I think it's time to take a step back and maybe revisit some of the approach here, but this change is in the right direction. Left two minor comments but otherwise LGTM.

packages/compiler/src/shadow_css.ts Outdated Show resolved Hide resolved
packages/compiler/test/shadow_css/host_and_host_context_spec.ts Outdated Show resolved Hide resolved
@JoostK JoostK added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews risky Identifies a PR that has a level of risk to merging action: global presubmit The PR is in need of a google3 global presubmit and removed action: review The PR is still awaiting reviews from at least one requested reviewer action: presubmit The PR is in need of a google3 presubmit labels Jun 20, 2023
@JoostK
Copy link
Member

JoostK commented Jun 20, 2023

Labelling as risky since this might affect rendering results in a subtle way. I recommend syncing this separately once TGP comes back green.

@JoostK
Copy link
Member

JoostK commented Jun 21, 2023

@JeanMeche could you also fix the typo in the commit message (attributes is misspelled)

@JeanMeche
fix(compiler): apply style on :host attributes in prod builds.
4ee1138 
In prod builds, selectors are optimized and spaces a removed. angular#48558 introduced a regression on selectors without spaces. This commit fixes tihs.

Fixes angular#49100
@JoostK JoostK removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Aug 4, 2023
@JeanMeche JeanMeche mentioned this pull request Sep 12, 2023
Closed
@dylhunn dylhunn modified the milestones: Backlog, v17-final Oct 10, 2023
@dylhunn dylhunn self-assigned this Oct 10, 2023
@dylhunn
Copy link
Contributor

dylhunn commented Oct 10, 2023 •
edited
Loading

One more presubmit in cl/572379101 -- presubmit is green.
Edit: actually, this needs a global as well. I will run it tonight.

TGP is running: https://test.corp.google.com/OCL:572379101:BASE:572432933:1696991069679:95503e21
Deflake run: https://fusion2.corp.google.com/presubmit/572379101/OCL:572379101:BASE:572432933:1696998499938:6d790136/targets

@dylhunn
Copy link
Contributor

dylhunn commented Oct 11, 2023 •
edited
Loading

The global presubmit shows one broken cloud console test. Very frustrating, to be blocked on a single breakage. I have reached out to the code owners to try to get their help fixing it.

@dylhunn
Copy link
Contributor

dylhunn commented Oct 11, 2023 •
edited
Loading

The owning team confirms the g3 failure as a flake.

Caretaker: TGP is green, good to merge. I suggest syncing on its own.

@dylhunn dylhunn added action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: global presubmit The PR is in need of a google3 global presubmit labels Oct 11, 2023
@atscott atscott added target: rc This PR is targeted for the next release-candidate and removed target: patch This PR is targeted for the next patch release target: major This PR is targeted for the next major release labels Oct 11, 2023
@atscott
Copy link
Contributor

atscott commented Oct 11, 2023

This PR was merged into the repository by commit 0198d21.

atscott pushed a commit that referenced this pull request Oct 11, 2023
@JeanMeche @atscott
fix(compiler): apply style on :host attributes in prod builds. (#49118)
073ebfe 
In prod builds, selectors are optimized and spaces a removed. #48558 introduced a regression on selectors without spaces. This commit fixes tihs.

Fixes #49100

PR Close #49118
@atscott atscott closed this in 0198d21 Oct 11, 2023
@angular-automatic-lock-bot
Copy link

angular-automatic-lock-bot bot commented Nov 11, 2023

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 Nov 11, 2023
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@JeanMeche @ChellappanRajan
fix(compiler): apply style on :host attributes in prod builds. (angul…
137555b 
…ar#49118)

In prod builds, selectors are optimized and spaces a removed. angular#48558 introduced a regression on selectors without spaces. This commit fixes tihs.

Fixes angular#49100

PR Close angular#49118
@JeanMeche JeanMeche deleted the fix/48558 branch February 29, 2024 13:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@JoostK JoostK JoostK approved these changes

Assignees

@dylhunn dylhunn

Labels

action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note risky Identifies a PR that has a level of risk to merging target: rc This PR is targeted for the next release-candidate

Projects

None yet

Milestone

v17-final

Development

Successfully merging this pull request may close these issues.

CSS styles not applied if using attribute HostBinding and styling a DIV with the direct child selector in a production build

6 participants

@JeanMeche @dylhunn @alan-agius4 @JoostK @atscott @jessicajaniuk
Morty Proxy This is a proxified and sanitized view of the page, visit original site.