Ideally this would be available as a patch version in both v19 and v20
As per our policies, we only land feature fixes on the current version.

Also, even if this could considered a fix (to respect the spec), the fact the applications can have relied on the current implementation, changing the behavior is a breaking change as such cannot land in a minor. (see https://www.hyrumslaw.com/)

Ideally this would be available as a patch version in both v19 and v20
As per our policies, we only land feature fixes on the current version.

Also, even if this could considered a fix (to respect the spec), the fact the applications can have relied on the current implementation, changing the behavior is a breaking change as such cannot land in a minor. (see https://www.hyrumslaw.com/)

No problem 😊

I mean that's kind of on them for misusing the purpose of the shadowdom 😂 but I take your point

Seeing as you cannot predict what additional stylesheets were getting inserted into the component before, I'd be amazed (actually horrified) if people have built ui around expecting things to be there that shouldn't be.

I'll take a nosey at the tests later on, ran the tests for platform-browser before and they passed, so will dig into the failures

Edit - I have updated tests and added extra test cases, marked as breaking change

I ran a global test on Google's codebase and confirm that this is a breakage change that we won't land outside of a breaking change window.

I ran a global test on Google's codebase and confirm that this is a breakage change that we won't land outside of a breaking change window.

No problem, that makes sense. Thanks for verifying.

I've fixed the lint issue and the integration test btw that was checking the bundle size.

I'm curious @JeanMeche - what was the issue in the codebase? Were some shadowdom components relying on global styles (that were previously being inserted into the shadowdom)?

@ryan-bendel So unfortunately this is not a solution that will be able to land, as it's very breaking for current users that would involve a significant effort on our end to clean up. We know that the ShadowDom encapsulation is broken as it currently is, and we certainly do want to fix it. Unfortunately we aren't able to prioritize this at the moment because of other pressing issues and priorities. It's definitely on our backlog (I put it there, myself). We really do appreciate and acknowledge your passion for this and your effort here to try to provide a fix for others. We hope to be able to get to resolving this issue soon.

@thePunderWoman - Disappointing, but I understand your reasoning.

When you say current users, you mean end consumers of Angular? If so, that problem will exist no matter when you tackle it. And kicking the can down the road just means there would be more users in the future that will need to fix, not less?

Or are you solely talking about some internal tooling at Angular? Quite keen to understand what you see as a significant clean up for the Angular team? Angular material doesn't use ShadowDom? Struggling to find where you use ShadowDom that would cause issues for the Angular team themselves?

I've created a node module that patches the DomRenderer as a workaround. This doesn't feel great as it relies on no significant changes to the structure of that class on any upgrade, so very brittle - but does the job.

How about in the interim as a compromise, how would the team feel about a new Encapsulation method until you get around to fixing it? IsolatedShadowDom or something? That way people who are currently feeling the pain of the performance issues this causes get an option to resolve sooner, and in the future when you get around to tackling it properly you can merge the two Encapsulation classes with and make it part of the upgrade migration?

Would you be able to ask that to the team to see if that would be acceptable? Happy to do the work if it is

Thanks

I can totally understand wanting clarity. Essentially any sort of issue that would change how styles are applied ends up being very difficult for us to land, as people tend to code around the bug and then depend on it. So fixing the bug means breaking the way their app looks, even if the code is more correct. We unfortunately cannot just tell people they have to now fix their styles and just roll it out. Additionally with styles automatic migrations aren't possible. So this is the biggest limiting factor for us.

Even when it doesn't affect styles and layout, we've had to say no to very valid, correct fixes to things many times in the past due to the effort it would take to land the change. These are often things we'd love to see land and we really do want them fixed, but we lack the bandwidth to be able to do it. These kinds of fixes take a lot of manual effort from the team on top of the code change. Hopefully this helps understand a bit better.

If you put up a PR for the IsolatedShadowDom change, I can promise you we'll look at it. It may be the right fix, but we can't make any promises beyond that.

@thePunderWoman - thanks for coming back to me.

I'm slightly more confused now though 😆 - I agree it is a breaking change for users, and of course it's something you would realistically never be able to apply a migration to so it doesn't disrupt a users ui.

My confusion therefore is around if that is the case, then how could you ever fix it? You've put it in the backlog to tackle as something you'd like to fix, but as you say, that change would essentially force anyone upgrading to refactor a chunk of their ui. So that sounds more like a "wont fix" than "will fix later" if you're drawing a hard line at the expectations on a user?

I'll take a look at a new PR this week. I'd possibly take a look at migrations too, it would be nice (if possible) to make the "fixed" Shadowdom encapsulation the default variant, and perhaps the migration could move existing Shadowdom users onto something like "LegacyShadowDom" with a Deprecated flag? It would hopefully nudge users in the right direction then, and wouldn't break existing ui.

But i've no idea where any of that migration code lives, so i'll have to take a bit of time to poke about.

@ryan-bendel It is certainly not an impossible thing to fix. It just requires the commitment to doing the work of fixing people. Your approach of creating a new version of encapsulation and deprecating another may be a good solution. I can't speak to others on the team, but I'd personally be interested to see it.

@thePunderWoman - tagged you in the new PR with the migration we discussed

It should be non-breaking with the migration, but if that one is a no-go with the team, the only other thing I guess that would be an option is to create a new encapsulation method IsolatedShadowDom

To me personally, the fixed ShadowDom whilst preserving the old behaviour in LegacyShadowDom with an automatic migration feels nicer, and would nudge users in the right direction so there is (hopefully) less to do in the future for the Angular team

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.}