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

Implement new module shims #92

Merged
merged 4 commits into from May 12, 2017
Merged

Implement new module shims #92

merged 4 commits into from May 12, 2017

Conversation

ghost
Copy link

@ghost ghost commented Mar 1, 2017

@locks asked me to PR this.

@ghost ghost changed the title Implement new module shims [WIP] Implement new module shims Mar 1, 2017
Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

Awesome! I'm super excited to be moving forward here!

A few notes/comments:

  • Remove JSHint usage (if we want linting, we should use eslint).
  • Ensure that all modules which use the global Ember import it properly (seems like many/most do not at this point).
  • Add deprecations to usages of generateModule for all "old" shims other than ember.
  • Remove old shim for rsvp and jquery

@stefanpenner
Copy link
Contributor

should these be implemented in the ember-source addon itself?

@workmanw
Copy link

workmanw commented Mar 7, 2017

@martndemus Can you add a shim for Ember.String.isHTMLSafe? I have a PR to add it to the "classic shims". I'm assuming this will supercede my open PR if it is to land here. Just wanted to make sure that was covered because not having a shim for isHTMLSafe is currently blocking another PR of mine. :)

PR: #93

@ghost
Copy link
Author

ghost commented Mar 17, 2017

@rwjblue / @locks updated!

Notes:

  • Haven't added isHTMLSafe to the new imports as it isn't in the RFCs list. I'll add it if I get a blessing from a Ember god to do so.
  • Haven't deprecated the Ember.Test shims, as they don't have an analog in the new shims yet.

@workmanw
Copy link

@martndemus @rwjblue Hmm ... you're right isHTMLSafe is not in the RFC, but it's counterpart htmlSafe is. I think part of the problem is that isHTMLSafe landed in the release channel around the same time the RFC was first drafted and it just got missed for that reason.

I'm not sure what the procedure here is. The RFC has already been merged, but clearly isHTMLSafe is public API. Do we need to try to get the RFC amended to include this?

@msalahz
Copy link

msalahz commented Apr 25, 2017

Any updates about this ?

@Turbo87
Copy link
Member

Turbo87 commented Apr 26, 2017

would it be possible for the deprecation messages to explicitly mention the new path so that you don't have to look it up manually all the time?

@rwjblue
Copy link
Member

rwjblue commented Apr 26, 2017

Yeah, that does sound nice. We should also probably also have the deprecation link back to a section on the README here so that we can provide more "prosey" explanations...

@locks
Copy link
Contributor

locks commented Apr 26, 2017

I mentioned on Slack that we also have an automatic migration tool, which might be preferable to adding that logic here as well.
My Kingdom for a clickable link that calls the migrator 😂

@Turbo87 Turbo87 changed the title [WIP] Implement new module shims Implement new module shims May 12, 2017
@Turbo87 Turbo87 merged commit c9b234b into ember-cli:master May 12, 2017
@Turbo87
Copy link
Member

Turbo87 commented May 12, 2017

Merged as discussed in the Ember CLI team meeting yesterday. We will release this as a beta today and then continue from there.

@ghost ghost deleted the new-shims branch May 12, 2017 07:51
@ghost
Copy link
Author

ghost commented May 12, 2017

🎉

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.

None yet

6 participants