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

Extracts code from Ember and sets minimum ember-cli-babel #1

Merged
merged 1 commit into from
Nov 11, 2017

Conversation

locks
Copy link
Contributor

@locks locks commented Aug 5, 2017

  • Add Code
  • Add tests
  • Fix loc
  • Deprecate loc
  • Bump ember-cli-babel to version that supports @ember/string

@locks locks force-pushed the string-module branch 2 times, most recently from 1562e44 to d47ff81 Compare August 5, 2017 00:41
@rwjblue
Copy link
Member

rwjblue commented Aug 5, 2017

ATM I think the tests are probably using Ember.String and not our addon/index.js versions (due to babel polyfill for modules).

@locks locks changed the title Add code from ember.js, as well as tests v1.0.0 Sep 2, 2017
@locks locks changed the title v1.0.0 Extracts code from Ember and sets minimum ember-cli-babel Sep 2, 2017
@dwickern
Copy link

dwickern commented Oct 2, 2017

Where'd htmlSafe / isHTMLSafe go?

@locks
Copy link
Contributor Author

locks commented Oct 3, 2017

@ember/template

@rwjblue
Copy link
Member

rwjblue commented Oct 5, 2017

@locks - status?

@locks
Copy link
Contributor Author

locks commented Oct 5, 2017

@rwjblue cleaning all the relevant repos in the AM

addon/index.js Outdated
deprecate(
'loc is deprecated, use an internationalization or localization addon instead.',
false,
{ id: 'ember-string-utils.loc', until: '3.5.0', url: 'http://babeljs.io/docs/learn-es2015/#template-strings' }
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be a link to ember-intl or ember-i18n or some other page about the topic?

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.

Looks good for initial implementation.

We should:

  • Create an issue to update the implementation here to avoid duplicating code. e.g. when Ember has the string functionality in it, we do not need to include our own implementation.
  • Land this PR
  • Update to [email protected]
  • Change index.js to use name: '@ember/string' instead of at-ember-string

@locks
Copy link
Contributor Author

locks commented Nov 8, 2017

I'm updating ember-cli-babel and the index.js name since those are quick, and landing this.

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.

Hmm. I don’t think the changes in [email protected] actually allow this yet. The version checker is looking upwards through node_modules to find if a package is present, and I don’t think we actually handle the project itself being the package...

@locks locks merged commit d6fd587 into master Nov 11, 2017
@locks locks deleted the string-module branch November 11, 2017 09:44
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.

4 participants