[keycloak-dev] Moving to standardized Promises
Stan Silvert
ssilvert at redhat.com
Thu Oct 3 11:00:52 EDT 2019
On 10/3/2019 5:05 AM, Stian Thorgersen wrote:
> Great, thanks. Will get some folks that know TypeScript better than me to
> take a look at it.
It looks good. I went ahead and merged it.
The only thing that worries me a bit is the use of generic parameter
defaults. This feature was introduced in TypeScript 2.3, which was
released on 27 April 2017. So customers who are stuck on TypeScript 2.2
or earlier will need a workaround. I don't know if many/any customers
would be affected.
>
> On Wed, 2 Oct 2019 at 20:38, Jon Koops <jonkoops at gmail.com> wrote:
>
>> I've incorporated the code with the changes we discussed and it can be
>> reviewed. See: https://github.com/keycloak/keycloak/pull/6318. Still need
>> to update the documentation I'll get to that hopefully tomorrow, if not the
>> day after.
>>
>> On Wed, Oct 2, 2019 at 1:57 PM Jon Koops <jonkoops at gmail.com> wrote:
>>
>>> Thanks for the quick response. I'll see if I can get the changes in today.
>>>
>>> On Wed, Oct 2, 2019 at 1:50 PM Stian Thorgersen <sthorger at redhat.com>
>>> wrote:
>>>
>>>>
>>>> On Wed, 2 Oct 2019 at 13:37, Jon Koops <jonkoops at gmail.com> wrote:
>>>>
>>>>> As far as I am concerned the pull requests can be merged as-is. However
>>>>> we need to make sure to add a 'legacy' promiseType to cover the transition
>>>>> period where promiseType will default to 'native' to give users the ability
>>>>> to opt-out of the native promises whilst migrating their code.
>>>>>
>>>>> Depending on what you prefer we can do two things. We either merge the
>>>>> pull requests and make a new task to introduce a 'legacy' promiseType. Or I
>>>>> can update the pull requests that are open now. Let me know what you think.
>>>>>
>>>> Added https://issues.jboss.org/browse/KEYCLOAK-11616 - if you can
>>>> incorporate that in current PRs that would be great.
>>>>
>>>>
>>>>> Related to this discussion there are some files that seem duplicated as
>>>>> I can find the same type definitions for the Keycloak adapter in the new
>>>>> theme as almost an exact copy. What should be done with with these files if
>>>>> we make changes? (the specific file in question is
>>>>> themes/src/main/resources/theme/keycloak-preview/account/resources/app/keycloak-service/keycloak.d.ts)
>>>>>
>>>>> It also seems that in the respective classes that use said definition
>>>>> that the legacy promises are also still used. We should look into finding a
>>>>> way to migrate this code as well. Perhaps including Keycloak as an NPM
>>>>> dependency would make some sense here as well.
>>>>>
>>>> This is there temporarily for the new account console and the team
>>>> working on the new account console is already aware that they need to
>>>> remove this duplicate.
>>>>
>>>>
>>>>> I don't consider the above observation as part of the migration in
>>>>> question, however it should probably be discussed by the team as well.
>>>>>
>>>>>
>>>>> On Wed, Oct 2, 2019 at 10:54 AM Stian Thorgersen <sthorger at redhat.com>
>>>>> wrote:
>>>>>
>>>>>> Updated https://issues.jboss.org/browse/KEYCLOAK-9346 with sub-tasks
>>>>>> to cover what we agreed and moved your two tasks to sub-tasks of this issue.
>>>>>>
>>>>>> I think both your PRs can be merged now, with
>>>>>> https://issues.jboss.org/browse/KEYCLOAK-11608 covering a clearer
>>>>>> message around the deprecation. I can handle that, but need to discuss with
>>>>>> the team around how we go about this first.
>>>>>>
>>>>>> Not sure when we can make this switch, will discuss it with the team
>>>>>> on Friday and let you know.
>>>>>>
>>>>>> On Fri, 27 Sep 2019 at 13:58, Jon Koops <jonkoops at gmail.com> wrote:
>>>>>>
>>>>>>> Great! I completely agree that the points outlined are the way
>>>>>>> forward. Let's focus on getting the deprecation message into the next
>>>>>>> release together with the updated documentation. There are pull requests
>>>>>>> for these changes and I would greatly appreciate a review of them.
>>>>>>>
>>>>>>> I'll also modify the existing pull requests to incorporate the idea
>>>>>>> of a 'legacy' promise type and refer to it the documentation with some
>>>>>>> elaboration that in a future version of Keycloak the default will change to
>>>>>>> 'native' over 'legacy'.
>>>>>>>
>>>>>>> As for the timeline of the changing of the default to 'native' and
>>>>>>> eventual removal of 'legacy' what would be the preferred moment to make
>>>>>>> these changes?
>>>>>>>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
More information about the keycloak-dev
mailing list