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

Support "import { Promise } from 'rsvp';" #18

Merged
merged 2 commits into from
Jul 8, 2017

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Jul 8, 2017

Resolves part of #13

import { Promise } from 'rsvp';

instead of

import RSVP from 'rsvp';

const { Promise } = RSVP;

/cc @cibernox @stefanpenner

@Turbo87 Turbo87 requested a review from rwjblue July 8, 2017 11:46
Copy link
Collaborator

@cibernox cibernox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

@rwjblue
Copy link
Member

rwjblue commented Jul 8, 2017

FWIW, we discussed this at the ember core team meeting yesterday and @tomdale clarified that in his original RFC what he intended by import RSVP from 'rsvp'; was that the rsvp module would support the standard public API of RSVP.

According to the source this would essentially allow:

import {
  asap,
  cast,
  Promise,
  EventTarget,
  all,
  allSettled,
  race,
  hash,
  hashSettled,
  rethrow,
  defer,
  denodeify,
  configure,
  on,
  off,
  resolve,
  reject,
  map,
  async,
  filter
} from 'rsvp';

Though we may want to exclude some of those though...

@rwjblue rwjblue merged commit 9169532 into ember-cli:master Jul 8, 2017
@rwjblue rwjblue mentioned this pull request Jul 8, 2017
@Turbo87 Turbo87 deleted the rsvp-promise branch July 9, 2017 12:14
@Turbo87
Copy link
Member Author

Turbo87 commented Jul 9, 2017

@rwjblue I'm now wondering if this import path was correct... according to the conventions it probably should have been import Promise from 'rsvp/promise', but do those conventions apply to RSVP too?

@Turbo87 Turbo87 mentioned this pull request Jul 9, 2017
@rwjblue
Copy link
Member

rwjblue commented Jul 9, 2017

Hmm. Good point. I think the general idea is to support whatever RSVP's API is from our rsvp module. Can you do require('rsvp/promise') today?

@Turbo87
Copy link
Member Author

Turbo87 commented Jul 9, 2017

I don't think so. AFAIK the public API is using everything from require('rsvp').

@tomdale
Copy link
Contributor

tomdale commented Jul 13, 2017

I agree with @rwjblue here: we should try to reduce the API surface area of Ember by exposing dependencies as-is, rather than providing our own module API on top.

A good litmus test for this is: can I take imports and associated code from a webpack app that uses RSVP and copy and paste it into an Ember app and have it work? If yes, then I think we're #winning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants