folio-testing regressions: multiple copies of plugin-find-user and users

Are we satisfied with the understanding expressed above, that if a platform includes module has a dependency on a plugin that expresses a peerDependency on another module (i.e. ui-checkout depends on ui-plugin-find-users, ui-plugin-find-users has a peerDependency on ui-users) then The Right Way (indeed the only way) to handle this situation is to make ui-users a dependency in the platform?

Welp, I did some more research and concluded my old "a peerDep will pull things into the build if they aren't there already" research was bunk. As of NPM 3.0 peerDependencies are not auto-installed. The release notes are crystal clear about this:

peerDependencies no longer cause anything to be implicitly installed. Instead, npm will now warn if a packages peerDependencies are missing, but it's up to the consumer of the module (i.e. you) to ensure the peers get installed / are included in package.json as direct dependencies or devDependencies of your package.

More details at https://medium.com/@jacob.h.page/common-npm-mistakes-51bf8989079f.

From what I can tell, the scenario outlined way up top ("Suppose your setup doesn't have the Users app, but does have Checkout. Checkout uses ui-plugin-find-user, and that needs to pull in ui-users or it won't work.") is simply not supported by NPM. If we want to guarantee (1) no dupes of apps and (2) API compatibility between ui-plugin-find-user and the app it depends on, the only option is use a peerDependency and require that a platform include the compatible version of ui-users in its package.json file.

The alternative is to leave things as they are, with ui-users as a regular dependency with a sufficiently lenient semver string that we don't get incompatible versions. Reviewing this, I think the original problem was actually an over-specific dependency version on "1.0.0" when it should have been "^1.0.0". We've had that fix in place for months now without issue.

On some level I think I still object to plugin-find-user pulling in ui-users at all.

There is no point in rehearsing all this again. That decision was made long ago. The question is not whether, but how.

Everything else you say in your comment says to me that you're in agreement with 's assessment that we should shift to peer-dependency.

Zak, I think you should go ahead and do it.

If Jason wants to circle back around at the whole approach to embedding modules, then let's file a separate issue for that. It doesn't need to stop us from fixing this one.

Heh, objections. On some level I think I still object to plugin-find-user pulling in ui-users at all, especially now that we're factoring so much code out of it into reusable pieces anyway. Apps should be apps and libraries should be libraries and it seems kinda awkward to have one code base (indeed, one component!) try and be both. The problem with notes is an example, do we even really want the user picker to show notes, isn't that a bit heavy?

As for the peerDep vs full dependency, I've less of an opinion. But would note that it seems desirable for plugin-find-users to be able to use an old ui-users for its purposes if there's a new major release of ui-users that it hasn't been refactored for yet. The possibility of duplication really can be a feature, and we should specify our acceptable versions well so that webpack can do its job and remove redundancy where it can. Might be good to talk to ?

Pulling in as he commented on the PR but was never tagged here. Jason, now that we've done more research, do you have any objections to merging https://github.com/folio-org/ui-plugin-find-user/pull/2?