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

Reexport the built-in MetricsAdapters from the app folder #432

Merged
merged 1 commit into from
Apr 14, 2022
Merged

Reexport the built-in MetricsAdapters from the app folder #432

merged 1 commit into from
Apr 14, 2022

Conversation

Windvis
Copy link
Contributor

@Windvis Windvis commented Mar 12, 2022

This ensures that Embroider doesn't strip the needed built-in adapters out of the build when staticAddonTrees is enabled. We still strip out unused adapters manually.

Closes #316

This ensures that Embroider doesn't strip them out of the build when `staticAddonTrees` is enabled. We still strip out unused adapters manually.
@Windvis
Copy link
Contributor Author

Windvis commented Mar 12, 2022

I experimented with an @embroider/macros implementation but I couldn't get a version to work that supported stripping out the unused adapters on both a classic and Embroider optimized build, so I went for a more old school approach 😄.

Copy link
Contributor

@jherdman jherdman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks! I'm a little stunned at how easy this scenario was to re-enable. I admit I feel a bit like a goof for not thinking of this sooner! 😞

@jherdman jherdman merged commit 045c076 into adopted-ember-addons:master Apr 14, 2022
@Windvis Windvis deleted the support-embroider-optimized branch April 14, 2022 13:16
@Windvis
Copy link
Contributor Author

Windvis commented Apr 14, 2022

@jherdman haha don't be so hard on yourself. It took me a couple of hours experimenting with the @embroider/macros before realizing this setup should work just fine and is way simpler 😄.

Thanks for the merge!

@gwak
Copy link

gwak commented May 10, 2022

Hello @jherdman, are there plans to release a new version on npm with those changes ?

@jherdman
Copy link
Contributor

Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Embroider optimized
3 participants