[keycloak-dev] Moving to standardized Promises

[Jon Koops]

I've polished the PR for the documentation. It now includes a section about
our motivations in regard to deprecating and eventually removing support
for legacy promise types (see:
https://github.com/keycloak/keycloak-documentation/pull/742).

This change should resolve both KEYCLOAK-11436
<https://issues.jboss.org/browse/KEYCLOAK-11436> and KEYCLOAK-11608
<https://issues.jboss.org/browse/KEYCLOAK-11608>.

On Thu, Oct 3, 2019 at 11:05 AM Stian Thorgersen <sthorger at redhat.com>
wrote:

> Great, thanks. Will get some folks that know TypeScript better than me to
> take a look at it.
>
> 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?
>>>>>>>
>>>>>>