-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[FEATURE modernized-built-in-components] Tracking Issue #19270
Comments
eramod
added a commit
to eramod/ember.js
that referenced
this issue
Feb 4, 2021
…ation Tracking issue: emberjs#19270 More specifically, it enables the <Input> attribute deprecations introduced in emberjs#19218, fixes broken tests, and updates all test {{input}} invocations to use <Input> instead. Deprecate input arguments
eramod
added a commit
to eramod/ember.js
that referenced
this issue
Feb 4, 2021
…ation Tracking issue: emberjs#19270 More specifically, it enables the <Input> attribute deprecations introduced in emberjs#19218, fixes broken tests, and updates all test {{input}} invocations to use <Input> instead. Deprecate input arguments
chancancode
added a commit
that referenced
this issue
Feb 5, 2021
Instead of hard-coding a list of events to bind (copied from the `EventDispatcher` – which could be reopened and modified), this refactor uses a modifier to dynamically read the list from the `EventDispatcher` at runtime instead. Part of #19270
chancancode
added a commit
that referenced
this issue
Feb 5, 2021
Switches the <Textarea /> component to use the new implementation as well. Passes all existing tests. Part of #19270
chancancode
added a commit
that referenced
this issue
Feb 5, 2021
Deprecate all unsupported args (and "deopt" the component for now) according to RFC 707. Part of #19270
chancancode
pushed a commit
to eramod/ember.js
that referenced
this issue
Feb 6, 2021
…ation Tracking issue: emberjs#19270 More specifically, it enables the <Input> attribute deprecations introduced in emberjs#19218, fixes broken tests, and updates all test {{input}} invocations to use <Input> instead.
chancancode
added a commit
that referenced
this issue
Feb 6, 2021
Instead of hard-coding a list of events to bind (copied from the `EventDispatcher` – which could be reopened and modified), this refactor uses a modifier to dynamically read the list from the `EventDispatcher` at runtime instead. Part of #19270
chancancode
added a commit
that referenced
this issue
Feb 6, 2021
Switches the <Textarea /> component to use the new implementation as well. Passes all existing tests. Part of #19270
chancancode
added a commit
that referenced
this issue
Feb 6, 2021
Deprecate all unsupported args (and "deopt" the component for now) according to RFC 707. Part of #19270
chancancode
added a commit
that referenced
this issue
Feb 6, 2021
Deprecate all unsupported args (and "deopt" the component for now) according to RFC 707. Part of #19270
chancancode
added a commit
that referenced
this issue
Feb 6, 2021
Switches the <Textarea /> component to use the new implementation as well. Passes all existing tests. Part of #19270
chancancode
added a commit
that referenced
this issue
Feb 6, 2021
Deprecate all unsupported args (and "deopt" the component for now) according to RFC 707. Part of #19270
chancancode
added a commit
that referenced
this issue
Feb 6, 2021
Switches the <Textarea /> component to use the new implementation as well. Passes all existing tests. Part of #19270
chancancode
added a commit
that referenced
this issue
Feb 6, 2021
Deprecate all unsupported args (and "deopt" the component for now) according to RFC 707. Part of #19270
chancancode
added a commit
that referenced
this issue
Feb 6, 2021
chancancode
added a commit
that referenced
this issue
Feb 8, 2021
Deprecate all unsupported args (and "deopt" the component for now) according to RFC 707. Part of #19270
chancancode
added a commit
that referenced
this issue
Feb 13, 2021
chancancode
added a commit
that referenced
this issue
Feb 13, 2021
Refactor to extra shared code into internal component to pave way for implementing <LinkTo> with the new infrastructure. Part of #19270
chancancode
added a commit
that referenced
this issue
Feb 25, 2021
Instead of hard-coding a list of events to bind (copied from the `EventDispatcher` – which could be reopened and modified), this refactor uses a modifier to dynamically read the list from the `EventDispatcher` at runtime instead. Part of #19270
chancancode
added a commit
that referenced
this issue
Feb 25, 2021
Switches the <Textarea /> component to use the new implementation as well. Passes all existing tests. Part of #19270
chancancode
added a commit
that referenced
this issue
Feb 25, 2021
Deprecate all unsupported args (and "deopt" the component for now) according to RFC 707. Part of #19270
chancancode
added a commit
that referenced
this issue
Feb 25, 2021
chancancode
pushed a commit
that referenced
this issue
Feb 25, 2021
…trings Tracking issue: #19270 Get tests passing for no actions passed to inputs as strings deprecation
chancancode
added a commit
that referenced
this issue
Feb 25, 2021
chancancode
added a commit
that referenced
this issue
Feb 25, 2021
chancancode
added a commit
that referenced
this issue
Feb 25, 2021
chancancode
added a commit
that referenced
this issue
Feb 25, 2021
Refactor to extra shared code into internal component to pave way for implementing <LinkTo> with the new infrastructure. Part of #19270
chancancode
added a commit
that referenced
this issue
Feb 25, 2021
chancancode
added a commit
that referenced
this issue
Feb 25, 2021
@chancancode - Mind double checking that the right checkboxes are checked off? |
sly7-7
pushed a commit
to sly7-7/ember.js
that referenced
this issue
Mar 10, 2021
…ation Tracking issue: emberjs#19270 More specifically, it enables the <Input> attribute deprecations introduced in emberjs#19218, fixes broken tests, and updates all test {{input}} invocations to use <Input> instead.
sly7-7
pushed a commit
to sly7-7/ember.js
that referenced
this issue
Mar 10, 2021
Instead of hard-coding a list of events to bind (copied from the `EventDispatcher` – which could be reopened and modified), this refactor uses a modifier to dynamically read the list from the `EventDispatcher` at runtime instead. Part of emberjs#19270
sly7-7
pushed a commit
to sly7-7/ember.js
that referenced
this issue
Mar 10, 2021
Switches the <Textarea /> component to use the new implementation as well. Passes all existing tests. Part of emberjs#19270
sly7-7
pushed a commit
to sly7-7/ember.js
that referenced
this issue
Mar 10, 2021
Deprecate all unsupported args (and "deopt" the component for now) according to RFC 707. Part of emberjs#19270
sly7-7
pushed a commit
to sly7-7/ember.js
that referenced
this issue
Mar 10, 2021
sly7-7
pushed a commit
to sly7-7/ember.js
that referenced
this issue
Mar 10, 2021
…trings Tracking issue: emberjs#19270 Get tests passing for no actions passed to inputs as strings deprecation
sly7-7
pushed a commit
to sly7-7/ember.js
that referenced
this issue
Mar 10, 2021
sly7-7
pushed a commit
to sly7-7/ember.js
that referenced
this issue
Mar 10, 2021
sly7-7
pushed a commit
to sly7-7/ember.js
that referenced
this issue
Mar 10, 2021
sly7-7
pushed a commit
to sly7-7/ember.js
that referenced
this issue
Mar 10, 2021
…nt handlers Tracking issue: emberjs#19270 Enables the input and textarea event handlers deprecation introduced in emberjs#19218 and fixes resultant failing tests
sly7-7
pushed a commit
to sly7-7/ember.js
that referenced
this issue
Mar 10, 2021
Refactor to extra shared code into internal component to pave way for implementing <LinkTo> with the new infrastructure. Part of emberjs#19270
sly7-7
pushed a commit
to sly7-7/ember.js
that referenced
this issue
Mar 10, 2021
Beautiful work! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The remaining tasks, in no particular order (most of these can be parallelized)
<Textarea>
to be based on the new implementation<LinkTo>
Some guidance on each of these...
Merge #19218 (🔒 @rwjblue)
Should be good to go, as far as I can tell.
Enable the deprecations introduced in #19218
The PR introduced a few new deprecations, but they are disabled even when the feature flag is enabled, by hard coding the deprecation condition to
true
. In each case though, I left the actual condition in a JS comment.The reason that these deprecations were not enabled (behind the feature flag) as part of the original PR was that those features are used in the internal test suite, so enabling them would require a modest amount of work fixing up the existing tests.
The deprecations are:
@id
argument to is deprecated. Instead, please pass the attribute directly, i.e.<Input id={{...}} />
instead of<Input @id={{...}} />
or{{input id=...}}
. (id
is just an example here, this applies to other attributes as well.)@change
argument to is deprecated. This would have overwritten the internalchange
method on the component and prevented it from functioning properly. Instead, please use the {{on}} modifier, i.e.<Input {{on "change" ...}} />
instead of<Input @change={{...}} />
or{{input change=...}}
. (change
is just an example here, this applies to other event methods as well.)@click
argument to is deprecated. Instead, please use the {{on}} modifier, i.e.<Input {{on "click" ...}} />
instead of<Input @click={{...}} />
or{{input click=...}}
. (click
is just an example here, this applies to other event as well.)@key-down
argument to is deprecated. Instead, please use the {{on}} modifier, i.e.<Input {{on "keydown" ...}} />
instead of<Input @key-down={{...}} />
or{{input key-down=...}}
. (key-down
is just an example here, this applies to other virtual event as well.)In each case, we will need...
expectDeprecation
test helper and stuff. Naturally, this test will only pass with the feature enabled, so you can guard it with@feature(EMBER_MODERNIZED_BUILT_IN_COMPONENTS)
. There are some examples in [FEATURE modernized-built-in-components] First-pass implementation #19218.expectDeprecation
and add a new test for the replacement feature if there isn't one already. If the test is not specifically about the deprecated feature and is just incidentally using that feature, you can refactor the test to use the replacement feature instead.In general...
@feature(EMBER_MODERNIZED_BUILT_IN_COMPONENTS)
or@feature(!EMBER_MODERNIZED_BUILT_IN_COMPONENTS)
to allow them to run only when the feature is enabled/disabled, respectively.@test
.EMBER_MODERNIZED_BUILT_IN_COMPONENTS
from@ember/canary-features
and guard a specific part of the test method in a fine-grained manner, using a regularif (EMBER_MODERNIZED_BUILT_IN_COMPONENTS) { ... } else { ... }
.Address #19218 (comment)
This is probably semi-blocked until contextual modifiers have landed. (:lock: @pzuraq)
In order to address the concern, we'd probably have to move the registration of event handlers into JavaScript which requires writing some kind of modifier. #19269 adds an API for this, but until we have contextual modifiers we would have to make the modifier available in the global namespace using some "private" name like
{{-deprecated-input-event-handlers ...}}
or something.We can do that, but given that contextual modifiers are pretty close, we probably should just wait. If that didn't turn out to be the case we can 🔓 and revisit this.
Identify, implement and deprecate other classic component features
The implementation in #19218 accounted for the classic component features that are currently tested. There may be other classic component features that are not covered by the existing tests though.
For example, you can pass a function via the
didInsertElement
(i.e.<Input @didInsertElement=... />
or{{input didInsertElement=...}}
) and it would "work" by shadowing the method/hook on the class, because that's how arguments on classic components work. RFC #671 explained this in a bit more details.We need to do an audit on the classic component API and find cases like this. In each case, we can decide:
<Input @init=... />
– pretty sure it won't work because we require you to call super, or<Input @tagName=... />
– what do you want?!)<Input @didInsertElement=... />
falls into this area... I hope?!)For the first 2 cases, we probably won't have to do anything.
For the third case, we'll probably have to emulate it and issue a deprecation message. #19218 has plenty of examples for this. In general, the strategy is to try an isolate the deprecated feature in a monkey patch outside of the class, so we can just delete it once the feature is no longer supported. If this is not possible or too difficult, we can also "deopt" the component (see the next section).
For cases that falls somewhere in between, we need to make a judgement call on what to do.
Implement the "deopt"
As mentioned above, there are some features that would be extremely difficult or impossible to emulate. RFC #671 gave some examples – such as calling
.reopen
on the component class.The plan for this is to keep shipping the old classic component implementation and fallback ("deopt") to the old implementation when these features are used, with a loud deprecation warning of course. This can be accomplished by setting the
modernized
flag tofalse
on the component.There are two kinds of features that may want to trigger this deopt – static and non-static ones. Static features are something like
Input.reopen(...)
. Once this is performed, we must deopt allInput
components. The non-static features are when something passes an argument when invoking the component (say<Input @didInsertElement=... />
), in which case we can just deopt that on instance only.Something like this:
Submit the "part 2" deprecation RFC
The deprecations introduced in #19218 and any additional deprecations introduced from the work items above, as well as the equivalent deprecations for the
<LinkTo>
component, needs to go through the RFC process before they can be enabled. This is mentioned in RFC #671 as well.We don't have to wait until we finished implementing all the deprecations to submit the RFC though. This can happen in parallel with the implementation, but it does require us to at least do the audit and think through what are the features we need to deprecate.
Migrate
<Textarea>
to be based on the new implementationThis is going to be very similar to the work in #19218. However, the functionality is almost identical so it can likely share most of the code with the
Input
component (it's sort of like a third@type
). This can be either done with subclassing or extracting the common code.Refactor
<LinkTo>
This is again very similar to #19218, just for the
<LinkTo>
component instead.The text was updated successfully, but these errors were encountered: