embroider-build / embroider

Compiling Ember apps into spec-compliant, modern Javascript.
MIT License
339 stars 140 forks source link

Implement configure / configureDependencies hooks from V2 Addon RFC #1303

Open runspired opened 1 year ago

runspired commented 1 year ago

rfc spec: https://rfcs.emberjs.com/id/0507-embroider-v2-package-format/

The RFC specified a way for an addon to configure itself or its dependencies at build time of the consuming app. EmberData requires use of this to populate it's ownConfig, currently we do this configuration via the included hook https://github.com/emberjs/data/blob/35ff11dd76dc15b2df3be6e672e9753c70dc0e0f/packages/store/addon-main.js#L71

BoussonKarel commented 1 year ago

Yes please! :+1:

ef4 commented 1 year ago

It's helpful to have this example from ember-data so we can figure out the path forward here.

The main reason this isn't implemented yet is that I don't want to allow addons to lock us into current environments and implementations. For example, the ember-data code uses process.cwd, __dirname, and require.resolve. Those are all things I would absolutely try to break in this new feature, because I don't want addons to be dependent on node-isms. They should work in any ES-modules environment, including browsers or edge-hosted workers.

This is why we want to improve the declarative API, like the macros and also possibly arguments to these new configure hooks, so that they meet these needs. That decouples the addons from the implementation. They can ask "is this dependency present?" without knowing how dependencies resolution works in the current environment.

runspired commented 1 year ago

Largely agree.

Fwiw that's necessary to work around bugs in require.resolve with paths, the special resolve that macros uses, and bugs with package managers creation of isolated trees that incorrectly hide peer-deps.

In theory it's the same as dependencySatisfies or moduleExists, but will work correctly where those currently will not. Obviously long term the hope would be that module resolution and package managers become less buggy.

The only thing I'd expect to always have access to is the full ENV. Which incidentally we also use to work around a macros bug where isTesting is false for production tests when it shouldn't be, and isDevelopingApp is false when it should be true in development builds.

Everything else can ESM/whatever. It resolves data configs from data itself to generate the ownConfig.

runspired commented 1 year ago

I don't want addons to be dependent on node-isms. They should work in any ES-modules environment, including browsers or edge-hosted workers.

Here's where I disagree: A build plugin (because that's what this configuration hook effectively is, a build plugin) will ultimately always have need of access to its own dependencies and some knowledge of the environment it is in. ESM is probably a good constraint as not all envs support require (bun, deno, service-worker build concepts), but access to the ENV, the process, and the ability to escape and do pretty much anything are assumed.

Example, unplugin for vue assumes node https://github.com/sxzz/unplugin-vue/blob/main/src/core/main.ts

I don't think we should be in the habit of restricting things that don't need to be restricted. "we eval as esm" sounds great. "we actively try to prevent you from accessing the toolbox that would normally be at your fingertips" does not. I'm highly skeptical of artificially constraining configuration to levels unheard of in the broader ecosystem: we just started escaping one overly restrictive system, I don't want to live in another: especially not one that we think up on our own in the next few weeks or months.

A parting aside: the theory that builds could run in the browser or edge-hosted workers is ... interesting. I'd never want to ship most of the things you'd need to do a quality build to those environments. Those environments are great receivers of artifacts, constraining designs with the assumption they are going to become the generators of artifacts I suspect will only lead to a lot of pain, but I'd also not bet against folks trying.

jelhan commented 11 months ago

Ember Bootstrap needs this functionality as well. Ember Bootstrap supports different Bootstrap versions in parallel. @embroider/macros are used to eliminate code paths only required for other Bootstrap versions. ownConfig is set based on Bootstrap version being used. See https://github.com/ember-bootstrap/ember-bootstrap/blob/bf587838fe8ee6c667a780ca93f79b28537c6dec/index.js#L112-L120 for current implementation (v1 addon).