Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accept optional mapFilename config for rollup-app-reexports #1202

Merged
merged 1 commit into from
May 16, 2022

Conversation

dfreeman
Copy link
Contributor

@dfreeman dfreeman commented May 16, 2022

I brought this up in Discord the other day and didn't get any immediate vehement "no way" responses, so I'm going ahead and proposing it here.

Tl;dr: in @embroider/addon-dev's tooling for generating app reexports, I'd like to be able to define a mapping between the source and reexport paths for an entity instead of assuming they're exactly the same.

Background

For addons that expose many resolvable entities (components, helpers, services, etc), it can be helpful to somehow namespace those entities to make it clear to consumers where they're coming from when they see a component invocation or service injection.

Short of no-longer-the-future Batman namespacing, a lighter-weight way to do that is by grouping those entities together under a directory so that everything your addon exposes is clearly prefixed with an indicator of where it's from. Concretely, rather than directly having components/foo in my cool-addon package, I might have components/cool-addon/foo, and consumers would write <CoolAddon::Foo /> in their templates.

It's a bit cumbersome to have that extra directory in the canonical path for the implementation, though: it's an additional layer of nesting to navigate through in editors and code browsers, and you end up writing somewhat redundant import paths like cool-addon/components/cool-addon/foo. The latter point also becomes a bit more painful as first-class component templates pick up steam and consumers begin needing to care about the import paths of the components, helpers and modifiers they use.

To mitigate this in a v1 addon, I can put my reexport in app/components/cool-addon/foo.js while leaving my implementation in addon/components/foo.hbs, but since reexports are managed as part of build tooling for v2 addons, I don't have that flexibility.

This Change

This change adds an optional appReexports function that devs can pass to appReexports to remap the names of files emitted as part of their app-js content.

  plugins: [
    // ...
    addon.appReexports([
      'components/**/*.js',
      'services/**/*.js',
    ], {
      mapFilename: (name) => name.replace(/^(services|components)\//, '$1/cool-addon/'),
    }),
    // ...
  ],

By configuring my v2 cool-addon like the snippet above, I can provide a reasonably ergonomic form factor for consuming my Foo component unambiguously in a loose-mode template (<CoolAddon::Foo />) or a strict-mode one (import Foo from 'cool-addon/components/foo').

@NullVoxPopuli
Copy link
Collaborator

how close is this to "just authoring the app-js section of package.json yourself?" - why would we opt for this instead of that?

I 100% understand the desire to re-map the input modules to the app-re-exports. Something I did a lot of in v1 addons, that I would like to see "a" solution for, is for having helpers, modifiers, components all in non-default exports (or in the same file), and still properly re-exported in app-js somehow.

That said, I think the approach you have here is extensible enough for future extending, if need it

@ef4
Copy link
Contributor

ef4 commented May 16, 2022

but since reexports are managed as part of build tooling for v2 addons, I don't have that flexibility.

Plugins like appReexports appear explicitly in rollup.config.js so you can replace or remove them. Nothing stops a v2 addon author from manually managing an app directory and the corresponding app-js entries in package.json.

That said, this change seems fine. The main nudge I want to give people with this plugin is away from putting arbitrary code in the app tree (limiting to only reexports). If people choose to drop the plugin, they lose that nudge. This option lets more people stick with the plugin.

@ef4 ef4 merged commit fbbaad7 into embroider-build:main May 16, 2022
@dfreeman dfreeman deleted the remap-reexports branch May 16, 2022 15:31
@dfreeman
Copy link
Contributor Author

dfreeman commented May 16, 2022

how close is this to "just authoring the app-js section of package.json yourself?" - why would we opt for this instead of that?

Doing that by hand is of course possible, but to Ed's point that's far more free-form and invites a lot more abuse. Having to manage the app-js entries in package.json manually is also a step backwards DX-wise from where even v1 addons are today, whereas having the reexports generated automatically is one of the nice ergonomic improvements of the v2 tooling overall.

If people choose to drop the plugin, they lose that nudge. This option lets more people stick with the plugin.

Exactly. Thank you for merging!

@jamescdavis
Copy link
Contributor

Can this be documented somewhere? Maybe in https://github.com/embroider-build/embroider/blob/main/ADDON-AUTHOR-GUIDE.md?

@ef4 ef4 added the enhancement New feature or request label May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants