ember-rfc176-data icon indicating copy to clipboard operation
ember-rfc176-data copied to clipboard

Missing public API imports

Open Turbo87 opened this issue 8 years ago • 62 comments

According to #11 the following list of documented public globals APIs in Ember have no corresponding new module import paths yet:

ContainerProxyMixin
 Ember.FEATURES.isEnabled
Ember.MutableEnumerable
Ember.Namespace
Ember.NativeArray
Ember.Test
Ember.Test.QUnitAdapter
Ember.testing
Ember.VERSION
RouterService
Ember.Handlebars.Utils.escapeExpression
Ember.HTMLBars.template
Ember.Handlebars.compile

user hook:

Ember.onerror

Additionally the ember-cli-shims were exporting the following globals APIs which don't have corresponding new module import paths yet either:

Ember.destroy
Ember.SortableMixin

Turbo87 avatar Jul 05 '17 06:07 Turbo87

Ember.K is deprecated, don't think it should be included?

locks avatar Jul 05 '17 10:07 locks

good point, I didn't check the deprecated flag on those 😞

Turbo87 avatar Jul 05 '17 11:07 Turbo87

Ember.Binding and Ember.bind are also deprecated I believe.

rwjblue avatar Jul 05 '17 12:07 rwjblue

Ember.onerror is a user provideable hook (not something that Ember itself provides).

rwjblue avatar Jul 05 '17 12:07 rwjblue

Ember.registerDeprecationHandler Ember.registerWarnHandler

These are both wrong, they should be Ember.Debug.register*. The docs may need updating (since I know that is where you generated this list from).

rwjblue avatar Jul 05 '17 12:07 rwjblue

I didn't think that we exposed the RouterService on the global at all actually. We make it available in the default app registry, but I don't think we actually set a Ember.RouterService (we'll have to confirm).

rwjblue avatar Jul 05 '17 12:07 rwjblue

I've updated the list

I didn't think that we exposed the RouterService on the global at all actually. We make it available in the default app registry, but I don't think we actually set a Ember.RouterService (we'll have to confirm).

that's probably why it's not prefixed with Ember.

Turbo87 avatar Jul 05 '17 12:07 Turbo87

@Turbo87 @rwjblue This is the list of public APIs without an import path in the new modules. However, some of those public API might be deprecated.

Is the intention of the RFC to expose all the public APIs as modules, regardless of their deprecation state, or those should not be included?

cibernox avatar Jul 05 '17 13:07 cibernox

Is the intention of the RFC to expose all the public APIs as modules, regardless of their deprecation state, or those should not be included?

I think it would need to be a case by case basis on the deprecated items. For example, I don't think K makes much sense (and you have already written a codemod for it).

rwjblue avatar Jul 05 '17 13:07 rwjblue

@rwjblue https://github.com/ember-cli/ember-rfc176-data/issues/12#issuecomment-313084371 up for review:

  • deprecation https://github.com/emberjs/ember.js/pull/15456/files
  • warn https://github.com/emberjs/ember.js/pull/15457/files

locks avatar Jul 05 '17 14:07 locks

Added Ember.defineProperty to the list.

rwjblue avatar Jul 06 '17 18:07 rwjblue

Proposals that try to follow the guidelines in the RFC.

Before After
Ember.ApplicationInstance import ApplicationInstance from "@ember/application-instance"
Ember.ApplicationInstance.BootOptions Is this public API?
Ember.ComputedProperty import ComputedProperty from "@ember/object/computed"
Ember.CoreObject import CoreObject from "@ember/object/core"
Ember.Debug Not exported, it's just a namespace
Ember.Debug.registerDeprecationHandler import { registerDeprecationHandler } from "@ember/debug"
Ember.Debug.registerWarnHandler import { registerWarnHandler } from "@ember/debug"
Ember.defineProperty No clue
Ember.Engine import Engine from "@ember/engine"
Ember.EngineInstance import Engine from "@ember/engine-instance"
Ember.Error import Error from "@ember/error"
Ember.FEATURES.isEnabled import { isEnabled } from "@ember/features";
Ember.getEngineParent import { getEngineParent } from "@ember/engine"
Ember.getWithDefault import { getWithDefault } from "@ember/object"
Ember.MutableEnumerable import MutableEnumerable from "@ember/mutable-enumerable
Ember.Namespace import Namespace from "@ember/object/namespace"
Ember.NativeArray Is there any reason for this to be public? Isn't this only used to extend the prototype of Array?
Ember.ObjectProxy @import ObjectProxy from "@ember/object-proxy"
Ember.Observable @import Observable from "@ember/object/observable"
Ember.PromiseProxyMixin @import PromiseProxyMixin from "@ember/promise-proxy-mixin"; (meh)
Ember.Test Not exported, just a namespace
Ember.Test.Adapter @import TestAdapter from "@ember/test/adapter"
Ember.Test.QUnitAdapter @import TestAdapter from "@ember/test/qunit-adapter" (Is this part of Ember? Shouldn't it be on an addon)
Ember.testing Unsure
Ember.VERSION import VERSION from "@ember/version" (this kind of breaks the rules of default exports being classes)
RouterService Unsure

Close seconds:

Before After
Ember.ApplicationInstance import ApplicationInstance from "@ember/application/instance"
Ember.EngineInstance import EngineInstance from "@ember/engine/instance"

cibernox avatar Jul 07 '17 16:07 cibernox

We discussed this issue at todays Ember core team meeting, and have generally agreed to your list above with the following (relatively small) modifications:

// some that you were unsure of
import { isTesting, setTesting } from '@ember/test'; // Ember.testing
import { defineProperty } from '@ember/object'; // Ember.defineProperty

// we preferred alternates for these:
import ApplicationInstance from "@ember/application/instance";
import EngineInstance from "@ember/engine/instance";
import PromiseProxyMixin from "@ember/object/promise-proxy-mixin";
import ObjectProxy from "@ember/object/proxy"

// we want to hold off on these for now (will review again next week):
Ember.VERSION
Ember.Test.QUnitAdapter // as of QUnit 2.0 this is completely provided by ember-qunit

Would love a PR adding the missing things, then we can update the listing in the description...

rwjblue avatar Jul 07 '17 19:07 rwjblue

instead of import MutableEnumerable from "@ember/mutable-enumerable" I would like to suggest import MutableEnumerable from "@ember/enumerable/mutable"

Turbo87 avatar Jul 08 '17 12:07 Turbo87

Updated table with suggested changes:

Before After
Ember.ApplicationInstance import ApplicationInstance from "@ember/application/instance"
Ember.ApplicationInstance.BootOptions Is this public API?
Ember.ComputedProperty import ComputedProperty from "@ember/object/computed"
Ember.CoreObject import CoreObject from "@ember/object/core"
Ember.Debug Not exported, it's just a namespace
Ember.Debug.registerDeprecationHandler import { registerDeprecationHandler } from "@ember/debug"
Ember.Debug.registerWarnHandler import { registerWarnHandler } from "@ember/debug"
Ember.defineProperty import { defineProperty } from "@ember/object"
Ember.Engine import Engine from "@ember/engine"
Ember.EngineInstance import Engine from "@ember/engine/instance"
Ember.Error import Error from "@ember/error"
Ember.FEATURES.isEnabled import { isEnabled } from "@ember/features";
Ember.getEngineParent import { getEngineParent } from "@ember/engine"
Ember.getWithDefault import { getWithDefault } from "@ember/object"
Ember.MutableEnumerable import MutableEnumerable from "@ember/mutable-enumerable
Ember.Namespace import Namespace from "@ember/object/namespace"
Ember.NativeArray Is there any reason for this to be public? Isn't this only used to extend the prototype of Array?
Ember.ObjectProxy @import ObjectProxy from "@ember/object/proxy"
Ember.Observable @import Observable from "@ember/object/observable"
Ember.PromiseProxyMixin @import PromiseProxyMixin from "@ember/object/promise-proxy-mixin"; (meh)
Ember.Test Not exported, just a namespace
Ember.Test.Adapter @import TestAdapter from "@ember/test/adapter"
Ember.Test.QUnitAdapter (Is this part of Ember? Shouldn't it be on an addon?)
Ember.testing import { isTesting, setTesting } from "@ember/test"
Ember.VERSION import VERSION from "@ember/version" (this kind of breaks the rules of default exports being classes)
RouterService Unsure

cibernox avatar Jul 08 '17 12:07 cibernox

Now that some of these have been landed, we should update the list in the description...

rwjblue avatar Jul 08 '17 12:07 rwjblue

@Turbo87 Can you edit the table and remove what was implemented? Heading off right now.

cibernox avatar Jul 08 '17 12:07 cibernox

I found another missing API: Ember.Test.registerAsyncHelper

cibernox avatar Jul 08 '17 14:07 cibernox

Missing API imports and they suggested import paths

Before After
Ember.Test.registerAsyncHelper @import { registerAsyncHelper } from '@ember/test
Ember.Test.registerHelper @import { registerHelper } from '@ember/test
Ember.Test.registerWaiter @import { registerWaiter } from '@ember/test
Ember.Test.unregisterHelper @import { unregisterHelper } from '@ember/test (what is the use case for this?)
Ember.Test.unregisterWaiter @import { unregisterWaiter } from '@ember/test (what is the use case for this?)

If there is agreement I could PR that tonight or tomorrow.

cibernox avatar Jul 08 '17 15:07 cibernox

Ya, those look good @cibernox

rwjblue avatar Jul 08 '17 17:07 rwjblue

What is the path forward with import { isTesting, setTesting } from '@ember/test';? This doesn't seem like something that can be defined using just the json manifest.

cibernox avatar Jul 08 '17 22:07 cibernox

I just noticed another one:

Ember.Handlebars.Utils.escapeExpression

Turbo87 avatar Jul 09 '17 10:07 Turbo87

one more interesting thing:

Ember.String.singularize
Ember.String.pluralize

These methods are apparently monkey-patched onto Ember.String by ember-inflector which is imported by ember-data

Turbo87 avatar Jul 09 '17 11:07 Turbo87

@Turbo87 I'm not sure ember should export that if it's not always there and depends on the inclusion of a library. If it is dependent on ember-inspector, it sounds to me that it should be import { pluralize, singularize } from 'ember-inspector'.

cibernox avatar Jul 09 '17 19:07 cibernox

Yes, confirm. We should add a deprecation in ember-inflector for that global...

rwjblue avatar Jul 09 '17 23:07 rwjblue

For libraries that Ember re-exports, can we stop re-exporting them for modules? For example, replace:

Ember.Handlebars.Utils.escapeExpression("<div></div>");

with

import Handlebars from "handlebars";
Handlebars.Utils.escapeExpression("<div></div>");

It sucks that Handlebars doesn't have a real module API other than exporting an instance as the default export from index.js AFAICT.


Update: I missed that we already discussed this in https://github.com/ember-cli/ember-rfc176-data/pull/18. :)

tomdale avatar Jul 13 '17 14:07 tomdale

I was editing the guides, and I don't see an export for Ember.libraries.register(libraryName, libraryVersion);.

locks avatar Jul 22 '17 11:07 locks

These are also some exports here that are marked private in API docs:

  • RSVP.off
  • Ember.AutoLocation
  • Ember.HashLocation
  • Ember.HistoryLocation
  • Ember.NoneLocation

There are also:

  • Ember.computed.filterProperty
  • Ember.computed.mapProperty

which are missing from API docs.

vlascik avatar Jul 27 '17 18:07 vlascik

Should we include Ember.Logger? I know @aklkv is having an issue with the codemod https://github.com/ember-cli/ember-modules-codemod/issues/28

Although I'm not entirely sure what the deprecation status of this is 🤔 @tomdale mentioned that the Ember.Logger is not required anymore because it was essentially a polyfill https://github.com/emberjs/rfcs/pull/176#issuecomment-272566327

I'm not sure how widespread this issue would be but I think we should have an upgrade path for anyone using Ember.Logger 👍

mansona avatar Jul 28 '17 09:07 mansona

Ember.onerror is a user provideable hook (not something that Ember itself provides).

Doesn't there still need to be an equivalent way to provide a callback for that hook in a world without the Ember global?

dfreeman avatar Jul 28 '17 17:07 dfreeman