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

Enable no-barrel imports Depreaciton #20813

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Dec 15, 2024

What are folks thoughts on enabling this? and then the deprecation messages would force us to work on improving the inspector experience?

I have some ideas for how to bring back compatibility.

Our setup would be something like this:

const EMBER_INSPECTOR_SUPPORT = Symbol.for('__EMBER_INSPECTOR__');
globalThis[EMBER_INSPECTOR_SUPPORT] ||= {};
globalThis[EMBER_INSPECTOR_SUPPORT].init = async function loadEmberInspectorUtilities() {
  // store things on the secret object 2 lines above
  globalThis[EMBER_INSPECTOR_SUPPORT].tracking = await import(...);
  /* ... etc ... */.container = await import(...);

  // ensuring to set up an event listener pair for communicating via postMessage with the inspector
}

This should probably wrap all of this code: https://github.com/emberjs/ember.js/pull/20775/files#diff-b2c569a1648b65b7314971b3bd201a82fda450a5cccc8c3d415b864166c98e27

And then on the app side, we only need to load the module, since everything the inspector needs is await-imported

import '@ember/inspector-support';

And then on the inspector side, we access the globalThis

// loading the inspector
await globalThis[EMBER_INSPECTOR_SUPPORT].init();

// we can now start building the UI / Panels in the ember-inspector project.

@PatrickJS , @ef4 thoughts?

@NullVoxPopuli NullVoxPopuli marked this pull request as ready for review December 15, 2024 17:41
@kategengler
Copy link
Member

Could you bring this comment to the RFC? That's really where enabled gets decided -- the 'Ready for Release' stage

@patricklx
Copy link
Contributor

patricklx commented Dec 15, 2024

There is already a wip pr for compatibility #20775 .
I think the minimal setup would be to just manually define the AMD modules inspector needs and trigger the 'ember' event so inspector nows when ember was loaded

@NullVoxPopuli
Copy link
Contributor Author

My preference would be to ditch AMD, even for the inspector 🙈

@patricklx
Copy link
Contributor

Then we can just work on #20775 :)
But well, removing the barrel is not about removing AMD. I think the only thing left that the inspector needs from the barrel import is actually the 'ember' event, at the very end of the barrel file. But there might be some more especially at the boot up code of inspector

@NullVoxPopuli
Copy link
Contributor Author

Then we can just work on #20775 :)

I'll review soon (Tomorrow?)

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