Skip to content

Conversation

@k-fish
Copy link
Member

@k-fish k-fish commented Feb 9, 2021

Summary

As mentioned in #3230, this is still causing problems for build systems. Pulled out the logic that makes the addon config available via an embroider macro to the 'config' hook for the addon, which gets rid of the conditional logic as well as pulling configPath which could be undefined. Tested (using the dummy app) on both sides of using Embroider and the existing build system and the macros appear to be populated now, so this should work.

As mentioned in #3230, this is still causing problems for build systems. Pulled out the logic to put config into an embroider macro to the 'config' hook for the addon, which gets rid of the conditional logic as well as pulling configPath which could be undefined. Tested on both sides of using Embroider and the existing build system and the macros appear to be populated now, so this should work.
@github-actions
Copy link
Contributor

github-actions bot commented Feb 9, 2021

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 20.27 KB (-0.01% 🔽)
@sentry/browser - Webpack 21.18 KB (0%)
@sentry/react - Webpack 21.2 KB (0%)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 27.36 KB (-0.01% 🔽)

@k-fish k-fish merged commit 6860b18 into master Feb 16, 2021
@k-fish k-fish deleted the fix/ember-embroider-backwards-compatibility-2 branch February 16, 2021 19:14
ahmedetefy pushed a commit that referenced this pull request Mar 10, 2021
* fix(ember): Fixing fetching config during build step

As mentioned in #3230, this is still causing problems for build systems. Pulled out the logic to put config into an embroider macro to the 'config' hook for the addon, which gets rid of the conditional logic as well as pulling configPath which could be undefined. Tested on both sides of using Embroider and the existing build system and the macros appear to be populated now, so this should work.
This was referenced Mar 14, 2021
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.

3 participants