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

fix(ember): Fix Ember to work with Embroider and Fastboot #3181

Merged
merged 6 commits into from
Jan 25, 2021

Conversation

k-fish
Copy link
Member

@k-fish k-fish commented Jan 18, 2021

Summary

This removes the ember-get-config dependency and opts to use Embroiders macros instead to have backwards and forwards compatibility.

Should fix #3174.

Other

  • Added embroider safe options to ember-try to ensure it works with the new build system
  • Allowed runtime options in lieu of sentry specific options.

This removes the ember-get-config dependency and opts to use Embroiders
macros instead to have backwards and forwards compatibility.
@k-fish k-fish force-pushed the fix/ember-embroider-fastboot branch from e207b88 to 0ba7163 Compare January 18, 2021 20:39
@github-actions
Copy link
Contributor

github-actions bot commented Jan 18, 2021

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 20.23 KB (+0.01% 🔺)
@sentry/browser - Webpack 21.1 KB (0%)
@sentry/react - Webpack 21.12 KB (0%)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 27.39 KB (-0.01% 🔽)

@kamilogorek
Copy link
Contributor

Lint and tests are failing (due to linting issue i guess).

@k-fish
Copy link
Member Author

k-fish commented Jan 19, 2021

@kamilogorek yeah left a line in by accident, 👍

@k-fish k-fish merged commit 96e3ef3 into master Jan 25, 2021
@k-fish k-fish deleted the fix/ember-embroider-fastboot branch January 25, 2021 21:18
@mydea
Copy link
Member

mydea commented Feb 2, 2021

Hmm, this breaks in my app now? Does this require a different configuration?

After looking through this change, it appears I have to set the config now in ember-cli-build.js like this:

 setConfig: {
        '@sentry/ember': {
          sentryConfig: {

is that correct? If so, this is not reflected in any changelog or the readme so far :)

@kamilogorek
Copy link
Contributor

cc @k-fish

@k-fish
Copy link
Member Author

k-fish commented Feb 2, 2021

@mydea thanks for bringing it up 👍 . You definitely shouldn't have to use setConfig, the macros addon is behaving differently between build systems for the contentFor addon hook it looks like, I'll put a fix up and it should be fixed in 6.0.5.

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.

Running sentry with Ember embroider and fastboot breaks build
3 participants