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

Add compat adapter for ember-get-config #770

Merged

Conversation

alexlafroscia
Copy link
Contributor

I've been struggling to get ember-get-config to work with the Vite packager due to the fact that it's desired behavior is to re-export a module from the host app.

At @ef4's suggestion I totally replaced the original way of doing this with a new approach that is Embroider-compatible. We can look up the host app's config at build-time and serialize it into the module's index.js; this maintains the add-on's API without having the export from the host app.

One drawback is that the config/environment contents are now serialized twice in the payload, but I don't think that's necessarily a huge deal.

One other approach I had tried was using the setOwnConfig/getOwnConfig API from the macros package but

  1. I couldn't get it working; I overrode the options getter and added the @embroider/macros property but getOwnConfig introduced into the module's index.js would return undefined
  2. It ultimately was just extra steps; the approach taken here is simpler by cutting out that whole step

@alexlafroscia alexlafroscia force-pushed the ember-get-config-compat-adapter branch from 4564c61 to 1a1295b Compare April 21, 2021 18:53
@alexlafroscia
Copy link
Contributor Author

alexlafroscia commented Apr 21, 2021

The CI failure seems like a browser timeout in both jobs, but I can't re-run CI myself

@alexlafroscia alexlafroscia force-pushed the ember-get-config-compat-adapter branch from 1a1295b to 4af7847 Compare April 25, 2021 17:46
@alexlafroscia
Copy link
Contributor Author

Rebased from master in hopes of getting a passing build this time around

@ef4 ef4 merged commit 64c72f5 into embroider-build:master Apr 29, 2021
@ef4
Copy link
Contributor

ef4 commented Apr 29, 2021

Thanks for working on this.

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.

3 participants