-
-
Notifications
You must be signed in to change notification settings - Fork 32
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
Change htmlSafe and isHTMLSafe import path #38
Conversation
7a40fad
to
3dff4aa
Compare
mappings.json
Outdated
"deprecated": false | ||
"deprecated": true, | ||
"replacement": { | ||
"module": "@ember/component", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ember/template
😉
3dff4aa
to
5078d8f
Compare
@Turbo87 re-👀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would like @rwjblue to verify though
See #37 (comment), we need the ability to have the same import map to different globals based on Ember version. Specifically, rolling this change out (mapping We need to have two entries for
|
Will there be a way to provide the current version of Ember to all the projects using this data (specially eslint plugin, and babel plugin)? |
I am quite worried about the complexity that will be introduced by this proposal. Especially since it's just working around the fact that we're still introducing new globals instead of using proper modules... |
@Turbo87 - Have I missed something in my statement above? Am I making it more complicated needlessly? |
probably not needlessly, I'm just worried about adding more and more responsibilities to this package |
The only other solution I can think of is splitting the data among different packages which I think would be worse, but open to other options. |
After rethinking here, I think we can actually a simpler thing. Leave the current changes in the PR alone ( @Serabe - Can you update this PR with those two new entries? |
I'll do it tonight. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this PR also add the @ember/template
equivalents?
edit: whoops, just noticed that that is what @rwjblue also suggested
Deprecates importing it from `@ember/string` and adds the replacement to `@ember/template`.
5078d8f
to
f9fdcc8
Compare
Rebased and updated. :( |
mappings.json
Outdated
} | ||
}, | ||
{ | ||
"global": "Ember._Template.htmlSafe", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I think this needs to stay Ember.String.htmlsafe
until we have a way to ensure that the transpilation will work for both [email protected] and [email protected] (soon to include this global).
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
something like global: ["Ember._Template.htmlSafe", "Ember.String.htmlSafe"]
?
but if we're going to make the format more complicated we should definitely document the format better 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would change it and think how to do different transpilations depending on the version. Keeping an array would force us to transpile to something like:
var x = (Ember._Template && Ember._Template.htmlSafe) || (Ember.String && Ember.String.htmlSafe);
I rather keep two versions (0.x for Ember.String and 0.(x+y) for Ember._Template) than start doing that. In case we want to keep all in one version, I would rather see something like:
{
"global": [{ "version_selector": "<2.18.0", "global": "Ember.String.htmlSafe"}, { "version_selector": ">=2.18.0", "global": "Ember._Template.htmlSafe"}]
}
Deprecates importing it from
@ember/string
and adds the replacementto
@ember/template
.