@uppy/core: move Uppy class to its own module

[aduh95]

This would allow users to use Uppy without loading preact if they are not using any UIPlugin.

const { Uppy, BasePlugin } = require('@uppy/core');
// can be replaced with
const Uppy = require('@uppy/core/lib/Uppy.js');
const BasePlugin = require('@uppy/core/lib/BasePlugin.js');

Fixes: #3220

[Murderlon]

Is there no other way to keep the import @uppy/core instead of the full path for all imports?

[aduh95]

Is there no other way to keep the import @uppy/core instead of the full path for all imports?

Users don't have to use the full path, it's only for those who need/want to opt-out of importing UIPlugin (maybe because they don't want to setup tree-shaking).

[Murderlon]

Users don't have to use the full path, it's only for those who need/want to opt-out of importing UIPlugin (maybe because they don't want to setup tree-shaking).

Everyone wants to opt-out of importing UIPlugin unnecessarily, right? And from the looks of the issue the person did have tree-shaking setup and it still happened. Ideally I'm looking for a solution where we keep the user-facing imports exactly as is, but it doesn't import UIPlugin if you only import Uppy.

[aduh95]

If we remove UIPlugin from the exports of @uppy/core, it would be a breaking change for users who expect to have access to it from there.
If we lazy-load UIPlugin, that could work but is quite cumbersome and could also break some bundler as it's not a very common practice.

let UIPlugin;
Reflect.defineProperty(module.exports, 'UIPlugin', {
  enumerable: true,
  configurable: true,
  get() {
    return UIPlugin ??= require('./UIPlugin')
  },
  set(value) {
    UIPlugin = value
  }
})

IMHO the change in this PR is a lesser evil, unless we can think of another solution?

[Murderlon]

Thanks for explaining. I'm not knowledgeable on bundlers enough to know if there is another solution. I'm sure @goto-bus-stop would know if there is, but they are on holidays.

It's just unfortunate we did a big 2.0 launch blog post with the selling point of not bundling Preact if you import BasePlugin, and yet that happens, unless you know about this potential import trick. We would have to change the post and the docs temporarily and then change the docs back once we do 3.0 with ESM.

Perhaps good to let this sit till @arturi and @goto-bus-stop are back and then make a decision? Or is this really the only feasible way forward?

[aduh95]

I think we should land this nonetheless, as isolating classes in their own module is good practice anyway, and that doesn't mean we can't find a better solution in the future. Holding on this PR doesn't do our users any good, landing it doesn't make the core problem go away but at least provides a workaround.

[Murderlon]

Fair enough! It would be good to add notes about these imports to the blog post and uppy core docs in this PR.

[Murderlon]

The migration guide should also be updated. And perhaps a note at the first code snippet here as well?

[arturi]

It's just unfortunate we did a big 2.0 launch blog post with the selling point of not bundling Preact if you import BasePlugin, and yet that happens, unless you know about this potential import trick. We would have to change the post and the docs temporarily and then change the docs back once we do 3.0 with ESM.

Yeah, it is unfortunate, but I guess that’s the way to go for now. At least with this PR we have that option to offer — Preact won’t be bundled via require('@uppy/core/lib/BasePlugin.js'). I am not a fan of lazy-loading here, I’d rather wait for 3.0. So I propose we merge this when you feel it’s ready and update the docs / blog post.