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

Doesn't work under embroider #133

Open
Tracked by #93
NullVoxPopuli opened this issue Oct 3, 2021 · 6 comments
Open
Tracked by #93

Doesn't work under embroider #133

NullVoxPopuli opened this issue Oct 3, 2021 · 6 comments

Comments

@NullVoxPopuli
Copy link
Contributor

https://github.com/GavinJoyce/ember-headlessui/pull/93/checks?check_run_id=3783717513#step:7:180
The error in the github action run:

not ok 1 Chrome 94.0 - [undefined ms] - Global error: Uncaught ReferenceError: Ember is not defined at http://localhost:7357/assets/vendor.js, line 1053

The code that's causing it:
https://github.com/mixonic/ember-cli-deprecation-workflow/blob/master/vendor/ember-cli-deprecation-workflow/main.js#L33

  let registerDeprecationHandler = require.has('@ember/debug') ? require('@ember/debug').registerDeprecationHandler : Ember.Debug.registerDeprecationHandler;

Why is the ember global accessed here? it needs to be imported.

@ef4
Copy link
Contributor

ef4 commented Jan 15, 2022

The easiest solution here is to drop support for old ember versions and say import { registerDeprecationHandler } from '@ember/debug';

Alternatively, instead of require.has use the dependencySatisfies from @embroider/macros.

@mixonic
Copy link
Member

mixonic commented Jan 15, 2022

The file in question is not in the addon tree, and that is why a plain import cannot be used.

This pr #117 explored adding a setup step in app.js and using the addon tree. There are some tradeoffs with the approach to consider but I'm not against it.

I don't think you need to drop any Ember versions to do this.

@ef4
Copy link
Contributor

ef4 commented Jan 16, 2022

I mentioned ember version support because I assumed that's what this was doing. I guess this is in vendor so it runs early enough?

Accessing ember api from vendor scripts is problematic going forward. Not just because of embroider -- if we're going to follow the ES module spec then ember's modules need to be accessed as modules. So either statically imported from a module (not from a script) or dynamically imported.

Asking apps to import something from app.js is definitely one option. Another is to add a separate <script type="module"> before the app's.

HeroicEric added a commit to HeroicEric/ember-power-select that referenced this issue Aug 15, 2023
This fixes the `embroiderOptimized` ember-try scenario that runs as part
of CI by removing the `ember-cli-deprecation-workflow` addon.

There's [an issue with ember-cli-deprecation-workflow](ember-cli/ember-cli-deprecation-workflow#133)
that causes an error during CI for the embroider optimized build.

I think we can remove `ember-cli-deprecation-workflow` from this addon
because it does not seem to be doing anything useful but maybe I'm
missing something?

This was originally added in
cibernox@c10dd8e
as part of cibernox#1431 but
there is not really much context to go off of as far as why it was
added. It seems like maybe it was added while debugging GitHub Actions
and accidentally left laying around?

- ember-cli/ember-cli-deprecation-workflow#133
- https://github.com/embroider-build/embroider/tree/main/packages/test-setup#embroideroptimizedembertryconfig
HeroicEric added a commit to HeroicEric/ember-power-select that referenced this issue Aug 16, 2023
There's [an issue with ember-cli-deprecation-workflow](ember-cli/ember-cli-deprecation-workflow#133)
that causes an error during CI for the embroider optimized build.

I think we can remove `ember-cli-deprecation-workflow` from this addon
because it does not seem to be doing anything useful but maybe I'm
missing something?

This was originally added in
cibernox@c10dd8e
as part of cibernox#1431 but
there is not really much context to go off of as far as why it was
added. It seems like maybe it was added while debugging GitHub Actions
and accidentally left laying around?

- ember-cli/ember-cli-deprecation-workflow#133
- https://github.com/embroider-build/embroider/tree/main/packages/test-setup#embroideroptimizedembertryconfig
@amk221
Copy link

amk221 commented Jan 19, 2024

Is there any way to get Embroider apps to build with ember-cli-deprecation-workflow? Or do I have to temporarily uninstall it?

@aklkv
Copy link

aklkv commented Jan 19, 2024

This worked for us as a temporary solution

@enspandi
Copy link

We're working with this branch currently https://github.com/lolmaus/ember-cli-deprecation-workflow/tree/ember-debug

    "ember-cli-deprecation-workflow": "github:lolmaus/ember-cli-deprecation-workflow#ember-debug",

But it requires a few changes you can check in the dummy app:

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

No branches or pull requests

6 participants