TypedData doesn't support config entity types; flag errors if you try

I agree that we should throw an exception for this, but I think that needs to happen a lot sooner.

Now with patch :)

5: 2228721-5.patch queued for re-testing.

Discussed a bit in IRC, we need to be able to create those definition objects, we just need to make sure they can't/don't return any property definitions because they don't have any. Another solution would be return an empty array instead of throwing an exception.. me and @Berdir couldn't figure out what would be the best way.

More discussions in IRC, @fago prefers to return an empty array when trying to get the property definitions of a config entity *and* throw an exception in EntityManager.

Yep, the EM method says it supports only content entities - so throwing an exception is totally fine there. However, EntityDataDefinition is for all kind of entities and an exception is not expected/defined by the interface. So the valid question of what kind of properties a config entity has should not thrown an exception. Of course integrating with config schema would be better, but this denotes the status quo.

Patch looks good to me.

Shouldn't we be checking for not content entity instead of is config entity, for example calling it with menu_link would still result in a fatal error...

The problem with checking for content entity is that there might be other entity types which don't extend ContentEntityInterface but use the typed data / field definitions system. How about this instead?

That will break with #2002138: Use an adapter for supporting typed data on ContentEntities.

We are checking ContentEntityInterface in a number of places, I'm not sure if you can get around not implementing that anymore...

I see. So any other entity type which wants to use the typed data system will have be a content entity, but it can still use a different storage handler. Ok then, color me convinced :)

Pesky unit tests :)

Thanks.

Hm. Unfortunate that we need to special-case here, but in talking to Tim, this is based on design decisions that were made back in the day. This patch only makes it explicit so people don't get fatal errors.

Committed and pushed to 8.x. Thanks!