-
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
Move helper disambiguation to a flag that is off by default #515
Changes from 1 commit
882c956
15a1365
e4b480a
37c3bce
208d697
ab2a93c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
<div>this template has no js </div> | ||
{{#bs-button type="primary"}}Primary{{/bs-button}} | ||
<BsButton @type="primary">Primary</BsButton> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above - curly invocation |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
<button> | ||
{{(titleize "string helpers for ember")}} | ||
{{titleize "string helpers for ember"}} | ||
</button> | ||
{{(biz-baz canConvert="no" why="helper" where="local")}} | ||
{{biz-baz canConvert="no" why="helper" where="local"}} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,9 @@ | ||
{{site-header user=this.user class=(if this.user.isAdmin "admin")}} | ||
<SiteHeader @user={{this.user}} @class={{if this.user.isAdmin "admin"}} /> | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above - curly invocation |
||
{{#super-select selected=this.user.country as |s|}} | ||
<SuperSelect @selected={{this.user.country}} as |s|> | ||
{{#each this.availableCountries as |country|}} | ||
{{#s.option value=country}}{{country.name}}{{/s.option}} | ||
<s.option @value={{country}}>{{country.name}}</s.option> | ||
{{/each}} | ||
{{/super-select}} | ||
</SuperSelect> | ||
|
||
{{ui/button text="Click me"}} | ||
<Ui::Button @text="Click me" /> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,12 +45,16 @@ function isBuiltInComponent(key) { | |
return BUILT_IN_COMPONENTS.includes(key); | ||
} | ||
|
||
function isNestedComponentTagName(tagName) { | ||
return ( | ||
tagName && | ||
tagName.includes && | ||
(tagName.includes('/') || (tagName.includes('-') && tagName.includes('.'))) | ||
); | ||
function isNestedComponentTagName(tagName, unambiguousHelpers = false) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't quite understand the need for this function to change, if we do need it then we should probably add a comment to leave some help for future us 😂 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reverted. |
||
if (unambiguousHelpers) { | ||
return ( | ||
tagName && | ||
tagName.includes && | ||
(tagName.includes('/') || (tagName.includes('-') && tagName.includes('.'))) | ||
); | ||
} | ||
|
||
return tagName && tagName.includes && (tagName.includes('/') || tagName.includes('-')); | ||
} | ||
|
||
function isWallStreet(tagName) { | ||
|
@@ -341,18 +345,21 @@ function isKnownHelper(fullName, config, invokableData) { | |
} | ||
|
||
if (isTelemetryData) { | ||
let isComponent = | ||
!config.helpers.includes(name) && | ||
[...(components || []), ...BUILT_IN_COMPONENTS].includes(name); | ||
if (config.unambiguousHelpers) { | ||
let isComponent = | ||
!config.helpers.includes(name) && | ||
[...(components || []), ...BUILT_IN_COMPONENTS].includes(name); | ||
|
||
if (isComponent) { | ||
return false; | ||
if (isComponent) { | ||
return false; | ||
} | ||
} | ||
|
||
let mergedHelpers = [...KNOWN_HELPERS, ...(helpers || [])]; | ||
let isHelper = mergedHelpers.includes(name) || config.helpers.includes(name); | ||
let isComponent = [...(components || []), ...BUILT_IN_COMPONENTS].includes(name); | ||
let strName = `${name}`; // coerce boolean and number to string | ||
return isHelper && !strName.includes('.'); | ||
return (isHelper || (!config.unambiguousHelpers && !isComponent)) && !strName.includes('.'); | ||
} else { | ||
return KNOWN_HELPERS.includes(name) || config.helpers.includes(name); | ||
} | ||
|
@@ -480,15 +487,19 @@ function transformToAngleBracket(fileInfo, config, invokableData) { | |
const isTagKnownHelper = isKnownHelper(tagName, config, invokableData); | ||
|
||
// Don't change attribute statements | ||
const isValidMustacheComponent = node.loc.source !== '(synthetic)' && !isTagKnownHelper; | ||
const isNestedComponent = isNestedComponentTagName(tagName); | ||
const isValidMustacheComponent = config.unambiguousHelpers | ||
? node.loc.source !== '(synthetic)' && !isTagKnownHelper | ||
: node.loc.source !== '(synthetic)' && !isKnownHelper(tagName, config, invokableData); | ||
|
||
const isNestedComponent = isNestedComponentTagName(tagName, config.unambiguousHelpers); | ||
|
||
if ( | ||
isValidMustacheComponent && | ||
(node.hash.pairs.length > 0 || node.params.length > 0 || isNestedComponent) | ||
) { | ||
return transformComponentNode(node, fileInfo, config); | ||
} else if ( | ||
config.unambiguousHelpers && | ||
isTagKnownHelper && | ||
node.path.type !== 'SubExpression' && | ||
walkerPath.parent.node.type !== 'AttrNode' && | ||
|
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.
This probably needs to go back to curly invocation for the input