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

Deprecate Ember.Namespace #376

Closed
runspired opened this issue Sep 18, 2018 · 21 comments
Closed

Deprecate Ember.Namespace #376

runspired opened this issue Sep 18, 2018 · 21 comments
Labels
S-Exploring In the Exploring RFC Stage Seeking Co-author T-framework RFCs that impact the ember.js library

Comments

@runspired
Copy link
Contributor

Largely a holdover from the globals era.

http://www.emberjs.com.cn/api/classes/Ember.Namespace.html

I believe it still has some use by the Ember Inspector for registering versions of libraries, but alternative ways of accessing library version information exist.

@locks locks added the T-framework RFCs that impact the ember.js library label Sep 18, 2018
@locks locks added this to Deprecations in Deprecation Candidates Sep 18, 2018
@Gaurav0
Copy link
Contributor

Gaurav0 commented Sep 20, 2018

Is DS from Ember Data an Ember.Namespace?

@rwjblue
Copy link
Member

rwjblue commented Sep 20, 2018

@Gaurav0 yes

@RobbieTheWagner
Copy link
Member

@runspired we're using it to get all the app containers I believe. https://github.com/emberjs/ember-inspector/blob/master/ember_debug/vendor/startup-wrapper.js#L161

Is there a better way to do this? As we try to add engines support, I'm also curious if we could add logic here to add in the different engines too?

@hjdivad
Copy link
Member

hjdivad commented Sep 26, 2018

Following on to @rwwagner90's comments, Ember.Namespace.NAMESPACES_BY_ID is extremely useful when debugging, as it gives you devtools console access to apps (and therefore the container, store &c.).

Any deprecation/removal here should retain some happy path for debugging IMHO.

@RobbieTheWagner
Copy link
Member

Agreed with @hjdivad, I think @runspired was saying this will be replaced with some new way to get the apps/containers, and as long as we retain that functionality, I am good with it 👍

@rwjblue
Copy link
Member

rwjblue commented Sep 27, 2018

While I completely agree with the overall point that @hjdivad made (we must provide public apis for console debugging), the actual mechanism that @hjdivad mentioned is 1) private api and 2) has a bunch of pitfalls. Accessing the “container” in this way exposes a very rough API that was never intended for access outside of the framework itself, we should instead provide a convienient public API for access the public owner APIs that we are familiar with.

@locks locks moved this from Deprecations to 4.0 in Deprecation Candidates Dec 7, 2020
@mixonic
Copy link
Sponsor Member

mixonic commented Jul 8, 2022

Howdy! We discussed this today at the framework meeting and think this issue is definitely ready to move into an RFC. Namespace should no longer be providing any unique functionality, not that we know of.

@RobbieTheWagner
Copy link
Member

@mixonic what are the public APIs that replace it? Is there a better way to do things like this? Ember.Namespace.NAMESPACES[1].__container__.lookup('service:foo')

@nightire
Copy link

@mixonic what are the public APIs that replace it? Is there a better way to do things like this? Ember.Namespace.NAMESPACES[1].__container__.lookup('service:foo')

getOwner(this).lookup('service:foo'); -> https://api.emberjs.com/ember/4.4/functions/@ember%2Fapplication/getOwner

@webark
Copy link

webark commented Jul 10, 2022

getOwner(this)

This would require individuals to include something in there code to hook up debugging. Maybe this is a higher level thing that Ember cli could do? But the current way ember inspector interacts with the ember app without any app code installed for debugging purposes would have to be changed.

@RobbieTheWagner
Copy link
Member

getOwner(this) only works when this is your application instance. I am asking for how to find the application instance in the console if you don't already have it.

@webark
Copy link

webark commented Jul 11, 2022

@rwwagner90 @mixonic if seems like if the desire is to not expose that at runtime, there would need to be some mechanism/package you could add which would expose that for debugging/inspection tools. And that should be a prerequisite for removing this api.

@NullVoxPopuli
Copy link
Sponsor Contributor

NullVoxPopuli commented Jul 11, 2022

during app boot you can do

// routes/application.js
import { getOwner } from '@ember/application';

export default class ApplicationRoute extends Route {
  beforeModel() {
    window.MY_APP ||= {}
    window.MY_APP.owner = getOwner(this);
  }  
}

and then anywhere in the console, you can do:

window.MY_APP.owner.lookup('whatever you want')

@webark
Copy link

webark commented Jul 11, 2022

Sure. which was what I meant by

This would require individuals to include something in there code to hook up debugging.

This would be a departure from the current functionality of ember inspector, and that transition should be laid out and implemented/documented before it is broken.

@RobbieTheWagner
Copy link
Member

Right, so this would mean apps would need to opt in to having their Application instance exposed?

@kellyselden
Copy link
Member

In our E2E tests, we look up a service to inspect feature flags. We would need do #376 (comment) to keep this functionality, which isn't bad.

@NullVoxPopuli
Copy link
Sponsor Contributor

ye! it could be implemented via base route, or a decorator.

a decorator that exposes your app name could look like this:

import Route from '@ember/routing/route';
import { getOwner } from '@ember/application';

export function exportAppAsGlobal(klass: Route) {
  return class extends klass {
    beforeModel() {
      let owner = getOwner(this);
      
      let config = owner.resolveRegistration('config:environment');
      let name = config.moduleName;
      
      window[name] = { owner };
      
      return super.beforeModel();
    }
  }
}

Usage:

@exportAppAsGlobal
export default class ApplicationRoute extends Route {
  // ...

I have not tested this, but it should get people movin

@webark
Copy link

webark commented Jul 12, 2022

It's not the technical lift that is at issue, it's pretty strait forward how to use the window global, it's the change in methodology that just needs to be paired with it, and communicated.

If it's an "inspector" addon you have to include, or something that is provided by ember-cli itself, I just think the story around that should be flushed out before this functionality is removed.

@RobbieTheWagner
Copy link
Member

I agree with @webark. If this requires opting in, it will remove the ability to inspect most Ember sites, which could be a feature, not a bug, if we want that locked down more, but I personally like being able to use Ember Inspector on any Ember site.

@locks locks added Seeking Co-author S-Exploring In the Exploring RFC Stage labels Jul 15, 2022
@wagenet
Copy link
Member

wagenet commented Jul 23, 2022

Looks like this is duplicated by #540.

@wagenet
Copy link
Member

wagenet commented Jul 29, 2022

Closing in favor of #832. We'll be sure to incorporate the discussion here into the future RFC. Thanks for your help!

@wagenet wagenet closed this as completed Jul 29, 2022
Deprecation Candidates automation moved this from 4.0 to Finished Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-Exploring In the Exploring RFC Stage Seeking Co-author T-framework RFCs that impact the ember.js library
Projects
No open projects
Development

No branches or pull requests