Remove LinkTo's tagName#19662
Closed
nlfurniss wants to merge 2 commits intoemberjs:masterfrom
nlfurniss:remove-link-to-tag-name
Closed
Remove LinkTo's tagName#19662nlfurniss wants to merge 2 commits intoemberjs:masterfrom nlfurniss:remove-link-to-tag-name
nlfurniss wants to merge 2 commits intoemberjs:masterfrom
nlfurniss:remove-link-to-tag-name
Conversation
mixonic
reviewed
Jul 20, 2021
| } | ||
| ); | ||
|
|
||
| this.modernized = false; |
Member
There was a problem hiding this comment.
It is asserted that passing <LinkTo @tagName="div"> still renders an <a> after this removal?
Contributor
Author
There was a problem hiding this comment.
Good call. I added an assert to the validateArguments method and some tests that assert for this error, but not sure if that's the right place to throw versus where @href throws
rwjblue
approved these changes
Jul 21, 2021
Member
|
Needs a rebase (one of the other PR's must have caused a conflict). |
mixonic
reviewed
Jul 22, 2021
Comment on lines
+83
to
+86
| assert( | ||
| 'Passing the `@tagName` argument to <LinkTo> is not supported.', | ||
| !('tagName' in this.args.named) | ||
| ); |
Member
There was a problem hiding this comment.
Two concerns:
With this implemented as an assert, what does that mean for production? Would a @tagName be applied in production, even if it the assert stopped you in development? Seems bad if so. Maybe it should be a throw.
But, further, IMO we don't need a check at all. I don't think we're planning to add asserts for every old component argument which was removed in 4.0. They were removed, which means passing one of those arguments (like @tagName) could just mean it is ignored. I can ask around on this second point, but the first concern about production behavior not applying the tag name needs to be confirmed.
Contributor
Author
There was a problem hiding this comment.
Yup, makes sense re: assert. As to the second point, classic components will keep tagName, so it seems like something special will need to be done for linkTo.
Member
|
FYI @nlfurniss I rebased and force pushed this branch. |
mixonic
requested changes
Jul 22, 2021
Member
mixonic
left a comment
There was a problem hiding this comment.
Per #19669 (comment) we should hold off on this until the built-in components are extracted. @chancancode has kicked off that work I believe.
This was referenced Jul 22, 2021
Merged
Merged
mixonic
added a commit
to mixonic/ember.js
that referenced
this pull request
Nov 4, 2021
* Remove the export of `@ember/component/checkbox` * Remove the export of `@ember/component/text-field` * Remove the export of `@ember/component/text-area` * Remove the export of `@ember/component/link-component` * Remove the assignment of `Ember.Checkbox` * Remove the assignment of `Ember.TextField` * Remove the assignment of `Ember.TextArea` * Remove the assignment of `Ember.LinkComponent` * Remove the assignment of `Ember.TextSupport` * Remove the assignment of `Ember.TargetActionSupport` * Hard-code `EMBER_MODERNIZED_BUILT_IN_COMPONENTS` to `true` path in most places. The remaining are a few spots where there are deprecations targeting *5.0*. * Hard-code the built-in components (`<Textarea>`, `<Input>`, `<LinkTo>`) for the modernized path * Remove the `query-params` helper (it is only needed by the legacy `LinkComponent` and was deprecated). * Review emberjs#19669 and ensure all `disabledWhen` is removed. * Review emberjs#19662 and ensure all `@tagName` support is removed * Review all `ember.built-in-components.*` deprecations and ensure their functionality is removed. * Remove code supporting `ember-glimmer.link-to.positional-arguments`, basically https://github.com/emberjs/ember.js/blob/master/packages/ember-template-compiler/lib/plugins/transform-link-to.ts
mixonic
added a commit
to mixonic/ember.js
that referenced
this pull request
Nov 4, 2021
* Remove the export of `@ember/component/checkbox` * Remove the export of `@ember/component/text-field` * Remove the export of `@ember/component/text-area` * Remove the export of `@ember/component/link-component` * Remove the assignment of `Ember.Checkbox` * Remove the assignment of `Ember.TextField` * Remove the assignment of `Ember.TextArea` * Remove the assignment of `Ember.LinkComponent` * Remove the assignment of `Ember.TextSupport` * Remove the assignment of `Ember.TargetActionSupport` * Hard-code `EMBER_MODERNIZED_BUILT_IN_COMPONENTS` to `true` path in most places. The remaining are a few spots where there are deprecations targeting *5.0*. * Hard-code the built-in components (`<Textarea>`, `<Input>`, `<LinkTo>`) for the modernized path * Remove the `query-params` helper (it is only needed by the legacy `LinkComponent` and was deprecated). * Review emberjs#19669 and ensure all `disabledWhen` is removed. * Review emberjs#19662 and ensure all `@tagName` support is removed * Review all `ember.built-in-components.*` deprecations and ensure their functionality is removed. * Remove code supporting `ember-glimmer.link-to.positional-arguments`, basically https://github.com/emberjs/ember.js/blob/master/packages/ember-template-compiler/lib/plugins/transform-link-to.ts
mixonic
added a commit
to mixonic/ember.js
that referenced
this pull request
Nov 4, 2021
* Remove the export of `@ember/component/checkbox` * Remove the export of `@ember/component/text-field` * Remove the export of `@ember/component/text-area` * Remove the export of `@ember/component/link-component` * Remove the assignment of `Ember.Checkbox` * Remove the assignment of `Ember.TextField` * Remove the assignment of `Ember.TextArea` * Remove the assignment of `Ember.LinkComponent` * Remove the assignment of `Ember.TextSupport` * Remove the assignment of `Ember.TargetActionSupport` * Hard-code `EMBER_MODERNIZED_BUILT_IN_COMPONENTS` to `true` path in most places. The remaining are a few spots where there are deprecations targeting *5.0*. * Hard-code the built-in components (`<Textarea>`, `<Input>`, `<LinkTo>`) for the modernized path * Remove the `query-params` helper (it is only needed by the legacy `LinkComponent` and was deprecated). * Review emberjs#19669 and ensure all `disabledWhen` is removed. * Review emberjs#19662 and ensure all `@tagName` support is removed * Review all `ember.built-in-components.*` deprecations and ensure their functionality is removed. * Remove code supporting `ember-glimmer.link-to.positional-arguments`, basically https://github.com/emberjs/ember.js/blob/master/packages/ember-template-compiler/lib/plugins/transform-link-to.ts
mixonic
added a commit
to mixonic/ember.js
that referenced
this pull request
Nov 4, 2021
* Remove the export of `@ember/component/checkbox` * Remove the export of `@ember/component/text-field` * Remove the export of `@ember/component/text-area` * Remove the export of `@ember/component/link-component` * Remove the assignment of `Ember.Checkbox` * Remove the assignment of `Ember.TextField` * Remove the assignment of `Ember.TextArea` * Remove the assignment of `Ember.LinkComponent` * Remove the assignment of `Ember.TextSupport` * Remove the assignment of `Ember.TargetActionSupport` * Hard-code `EMBER_MODERNIZED_BUILT_IN_COMPONENTS` to `true` path in most places. The remaining are a few spots where there are deprecations targeting *5.0*. * Hard-code the built-in components (`<Textarea>`, `<Input>`, `<LinkTo>`) for the modernized path * Remove the `query-params` helper (it is only needed by the legacy `LinkComponent` and was deprecated). * Review emberjs#19669 and ensure all `disabledWhen` is removed. * Review emberjs#19662 and ensure all `@tagName` support is removed * Review all `ember.built-in-components.*` deprecations and ensure their functionality is removed. * Remove code supporting `ember-glimmer.link-to.positional-arguments`, basically https://github.com/emberjs/ember.js/blob/master/packages/ember-template-compiler/lib/plugins/transform-link-to.ts
mixonic
added a commit
to mixonic/ember.js
that referenced
this pull request
Nov 8, 2021
* Remove the export of `@ember/component/checkbox` * Remove the export of `@ember/component/text-field` * Remove the export of `@ember/component/text-area` * Remove the export of `@ember/component/link-component` * Remove the assignment of `Ember.Checkbox` * Remove the assignment of `Ember.TextField` * Remove the assignment of `Ember.TextArea` * Remove the assignment of `Ember.LinkComponent` * Remove the assignment of `Ember.TextSupport` * Remove the assignment of `Ember.TargetActionSupport` * Hard-code `EMBER_MODERNIZED_BUILT_IN_COMPONENTS` to `true` path in most places. The remaining are a few spots where there are deprecations targeting *5.0*. * Hard-code the built-in components (`<Textarea>`, `<Input>`, `<LinkTo>`) for the modernized path * Remove the `query-params` helper (it is only needed by the legacy `LinkComponent` and was deprecated). * Review emberjs#19669 and ensure all `disabledWhen` is removed. * Review emberjs#19662 and ensure all `@tagName` support is removed * Review all `ember.built-in-components.*` deprecations and ensure their functionality is removed. * Remove code supporting `ember-glimmer.link-to.positional-arguments`, basically https://github.com/emberjs/ember.js/blob/master/packages/ember-template-compiler/lib/plugins/transform-link-to.ts
mixonic
added a commit
to mixonic/ember.js
that referenced
this pull request
Nov 8, 2021
* Remove the export of `@ember/component/checkbox` * Remove the export of `@ember/component/text-field` * Remove the export of `@ember/component/text-area` * Remove the export of `@ember/component/link-component` * Remove the assignment of `Ember.Checkbox` * Remove the assignment of `Ember.TextField` * Remove the assignment of `Ember.TextArea` * Remove the assignment of `Ember.LinkComponent` * Remove the assignment of `Ember.TextSupport` * Remove the assignment of `Ember.TargetActionSupport` * Hard-code `EMBER_MODERNIZED_BUILT_IN_COMPONENTS` to `true` path in most places. The remaining are a few spots where there are deprecations targeting *5.0*. * Hard-code the built-in components (`<Textarea>`, `<Input>`, `<LinkTo>`) for the modernized path * Remove the `query-params` helper (it is only needed by the legacy `LinkComponent` and was deprecated). * Review emberjs#19669 and ensure all `disabledWhen` is removed. * Review emberjs#19662 and ensure all `@tagName` support is removed * Review all `ember.built-in-components.*` deprecations and ensure their functionality is removed. * Remove code supporting `ember-glimmer.link-to.positional-arguments`, basically https://github.com/emberjs/ember.js/blob/master/packages/ember-template-compiler/lib/plugins/transform-link-to.ts
mixonic
added a commit
to mixonic/ember.js
that referenced
this pull request
Nov 9, 2021
* Remove the export of `@ember/component/checkbox` * Remove the export of `@ember/component/text-field` * Remove the export of `@ember/component/text-area` * Remove the export of `@ember/component/link-component` * Remove the assignment of `Ember.Checkbox` * Remove the assignment of `Ember.TextField` * Remove the assignment of `Ember.TextArea` * Remove the assignment of `Ember.LinkComponent` * Remove the assignment of `Ember.TextSupport` * Remove the assignment of `Ember.TargetActionSupport` * Hard-code `EMBER_MODERNIZED_BUILT_IN_COMPONENTS` to `true` path in most places. The remaining are a few spots where there are deprecations targeting *5.0*. * Hard-code the built-in components (`<Textarea>`, `<Input>`, `<LinkTo>`) for the modernized path * Remove the `query-params` helper (it is only needed by the legacy `LinkComponent` and was deprecated). * Review emberjs#19669 and ensure all `disabledWhen` is removed. * Review emberjs#19662 and ensure all `@tagName` support is removed * Review all `ember.built-in-components.*` deprecations and ensure their functionality is removed. * Remove code supporting `ember-glimmer.link-to.positional-arguments`, basically https://github.com/emberjs/ember.js/blob/master/packages/ember-template-compiler/lib/plugins/transform-link-to.ts
mixonic
added a commit
to mixonic/ember.js
that referenced
this pull request
Nov 9, 2021
* Remove the export of `@ember/component/checkbox` * Remove the export of `@ember/component/text-field` * Remove the export of `@ember/component/text-area` * Remove the export of `@ember/component/link-component` * Remove the assignment of `Ember.Checkbox` * Remove the assignment of `Ember.TextField` * Remove the assignment of `Ember.TextArea` * Remove the assignment of `Ember.LinkComponent` * Remove the assignment of `Ember.TextSupport` * Remove the assignment of `Ember.TargetActionSupport` * Hard-code `EMBER_MODERNIZED_BUILT_IN_COMPONENTS` to `true` path in most places. The remaining are a few spots where there are deprecations targeting *5.0*. * Hard-code the built-in components (`<Textarea>`, `<Input>`, `<LinkTo>`) for the modernized path * Remove the `query-params` helper (it is only needed by the legacy `LinkComponent` and was deprecated). * Review emberjs#19669 and ensure all `disabledWhen` is removed. * Review emberjs#19662 and ensure all `@tagName` support is removed * Review all `ember.built-in-components.*` deprecations and ensure their functionality is removed. * Remove code supporting `ember-glimmer.link-to.positional-arguments`, basically https://github.com/emberjs/ember.js/blob/master/packages/ember-template-compiler/lib/plugins/transform-link-to.ts
kategengler
pushed a commit
that referenced
this pull request
Nov 10, 2021
* Remove the export of `@ember/component/checkbox` * Remove the export of `@ember/component/text-field` * Remove the export of `@ember/component/text-area` * Remove the export of `@ember/component/link-component` * Remove the assignment of `Ember.Checkbox` * Remove the assignment of `Ember.TextField` * Remove the assignment of `Ember.TextArea` * Remove the assignment of `Ember.LinkComponent` * Remove the assignment of `Ember.TextSupport` * Remove the assignment of `Ember.TargetActionSupport` * Hard-code `EMBER_MODERNIZED_BUILT_IN_COMPONENTS` to `true` path in most places. The remaining are a few spots where there are deprecations targeting *5.0*. * Hard-code the built-in components (`<Textarea>`, `<Input>`, `<LinkTo>`) for the modernized path * Remove the `query-params` helper (it is only needed by the legacy `LinkComponent` and was deprecated). * Review #19669 and ensure all `disabledWhen` is removed. * Review #19662 and ensure all `@tagName` support is removed * Review all `ember.built-in-components.*` deprecations and ensure their functionality is removed. * Remove code supporting `ember-glimmer.link-to.positional-arguments`, basically https://github.com/emberjs/ember.js/blob/master/packages/ember-template-compiler/lib/plugins/transform-link-to.ts (cherry picked from commit 5d24ae7)
Member
|
Supplanted by #19806. Thank you @nlfurniss ! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #19617
Deprecation Guide
Tests pass locally