Skip to content

Conversation

@sunghwan2789
Copy link
Contributor

@sunghwan2789 sunghwan2789 commented Jan 3, 2025

DecorationException is thrown if a non-keyed service and a keyed service are registered.

I think this is a bug since 5.0.3

This PR fix Decorate not to try decorate keyed service

@khellang
Copy link
Owner

khellang commented Jan 4, 2025

Hmm, I keep staring at the code, but I'm having trouble groking what's actually changed here. Why is the string equality check not enough?

I see the cast probably should be pulled out as a pattern cast and only run the string equality check if the key is actually a string, but I'm having trouble seeing why the reference equality check is necessary. Is it an optimization? To catch nulls?

@khellang
Copy link
Owner

khellang commented Jan 4, 2025

Ah, after looking at the test (thanks for including it ❤️) I get it. There's a KeyedService.AnyKey sentinel object that's being checked for. I wasn't aware of this value.

Can we maybe include a comment to point out why those checks are there? 😄

@sunghwan2789
Copy link
Contributor Author

@khellang While adding a comment, I found that object.Equals is a better choice as it checks for ordinal string equality and can pass the test case as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants