Lightning Core has undeclared dependency on field_ui

[danepowell]

If you disable field_ui, you'll get a class not found exception thrown by a class in lightning_core: https://github.com/acquia/lightning/blob/8.x-2.x/modules/lightning_features/lightning_core/src/Form/EntityDisplayModeAddForm.php

At a minimum, this dependency should be declared to avoid a fatal error, which is what this PR does.

However, long-term I think this dependency should be eliminated, since best practice is typically to disable Field UI in production environments.

Note that while field_ui is a declared dependency of the parent Lightning profile, this is no longer adequate protection since sub-profiles can exclude it.

[danepowell]

I don't know why the pipelines job failed, but I can't imagine it's related to this change.

[balsama]

Yeah - phantomjs crashed on pipelines - on a known issue. Travis runs that same tests plus upgrade tests. So we're ok there.

This came up before. I thought something had possibly regressed, but I think previously field_ui was an undeclared dependency in panels. See this issue: https://www.drupal.org/node/2664682#comment-11786436

As far as long-term not making it a dependency, this might relate to the ongoing conversation about what dependencies of profile really are. IMO, they're the word dependency is a misnomer. It's more of "this is what should be enabled out of the box". But this PR adds it as a dependency to lightning_core, not the profile. So it would prevent you from uninstalling it.

@phenaproxima can we just make EntityDisplayModeAddForm a little smarter so it doesn't throw an exception without field_ui?

[phenaproxima]

I have another solution I think I would prefer -- let's just add a \Drupal::moduleHandler()->moduleExists('field_ui') around the logic in lightning_core_entity_type_alter() which uses the offending classes. That way, we can avoid adding a hard dependency.

[camoa]

Kinda did what @phenaproxima proposed, got it working at once.

I just changed my vendor lightning_core copy, so no patch, but wanted to report it works.

[phenaproxima]

Closing this PR to implement my suggested fix in another branch (on my own fork).

[balsama]

Fixed in #340