atls / nestjs

NestJS Custom Workshop
BSD 3-Clause "New" or "Revised" License
33 stars 8 forks source link

Update keto dependencies versions #280

Closed IgorSmugalov closed 9 months ago

IgorSmugalov commented 1 year ago

All dependencies updated to the last versions in the keto module Except:

TorinAsakura commented 1 year ago

конфликт

IgorSmugalov commented 1 year ago

@SlumberyDude сделал ребейз, не могу понять почему у меня продублировали коммиты, плюс при разрешении конфликтов с файлами pnp.cjc и yarn.lock выбирал вариант 'использовать свою версию' и почему-то в таком случае всегда натыкался на проблемы с тестами

SlumberyDude commented 1 year ago

@IgorSmugalov Да я сам в ребейзе не силен если честно, в этом плане советчик плохой. Ты после ребейза зафорсил пуш в ремот? У нас технология следующая (в теории) Например, нужно влиться в мастер и конфликт.

Пуш не проходит

Должно сработать, но у меня у самого с этой технологией проблемы ><

То есть чтобы не форсить пуш надо делать еще пулл.

По поводу дублирования не знаю, могу предположить что во время ребейза в ветку добавляются коммиты с ветки на которую ребейзишь.

Кстати, куда ты делаешь ребейз? На мастер? Сейчас вроде конфликтов нет и чеки проходят

IgorSmugalov commented 1 year ago

@SlumberyDude я как раз зафорсил через --amend pnp.cjc и yarn.lock чтобы прошли чеки еще бы разобраться с 2мя соседними моими PR, в какой момент я накосячил - понимаю, сделал ревёрт мерджа, но далее не хочу двигаться, так как не думаю, что смогу сам правильно разобраться

ArtemMakhaydinov commented 1 year ago

@TorinAsakura, @SlumberyDude Был дополнительно установлен пакет ts-morph, его требовал при обновлении @nestjs/graphql. Все успешно обновилось за исключением: "@nestjs/core": "10.1.3" При повышении чек выдает:

➤ YN0000: │ packages/keto/src/guards/keto-access-control.guard.ts
➤ YN0000: │
➤ YN0000: │ Error: Object is of type 'unknown'.
➤ YN0000: │
➤ YN0000: │
➤ YN0000: │ packages/keto/src/guards/keto-access-control.guard.ts
➤ YN0000: │
➤ YN0000: │ Error: Object is of type 'unknown'.
➤ YN0000: │
➤ YN0000: │
➤ YN0000: │ packages/keto/src/guards/keto-access-control.guard.ts
➤ YN0000: │
➤ YN0000: │ Error: Object is of type 'unknown'.
SlumberyDude commented 1 year ago

@ArtemMakhaydinov Покажи это место в коде на которое он ругается, может можно там типчик поправить?

ArtemMakhaydinov commented 1 year ago

@SlumberyDude @nestjs/graphql без ts-morph или @nestjs/core 10.2+ версии?

SlumberyDude commented 1 year ago

@ArtemMakhaydinov Если ts-morph нужен то можно его поставить в dev. Я про nestjs/core, что там за тип валится?

ArtemMakhaydinov commented 1 year ago

@SlumberyDude Если апнуть @nestjs/core, ide при билде выдает это и подчеркивает константу policy в указанном файле.

➤ YN0000: ┌ Library Build
➤ YN0000: │ packages/keto/src/guards/keto-access-control.guard.ts
➤ YN0000: │
➤ YN0000: │ Error: Object is of type 'unknown'.
➤ YN0000: │
➤ YN0000: │
➤ YN0000: │ packages/keto/src/guards/keto-access-control.guard.ts
➤ YN0000: │
➤ YN0000: │ Error: Object is of type 'unknown'.
➤ YN0000: │
➤ YN0000: │
➤ YN0000: │ packages/keto/src/guards/keto-access-control.guard.ts
➤ YN0000: │
➤ YN0000: │ Error: Object is of type 'unknown'.
SlumberyDude commented 1 year ago

@ArtemMakhaydinov там у policy поменялся тип на unknown? Можешь предложить исправление?

ArtemMakhaydinov commented 1 year ago

@SlumberyDude при объявлении policy в вызове метода объявлен дженерик , я не силен в TS, но мне казалось, что при вызовах функций/методов, тайп не нужно указывать. Без этого все нормально билдится, чекается и тестится на последней версии @nestjs/core. Вот эта строка(в изначальном виде):

const policy = this.reflector.get<any>(ACCESS_POLICY_METADATA, context.getHandler())
SlumberyDude commented 1 year ago

@ArtemMakhaydinov Вроде приемлемо. Ты кстати тег details закрываешь неправильно, ставь слеш перед details, а то у тебя иногда вложенные списки получаются)

TorinAsakura commented 1 year ago

281 (review)

проблема не решена

SlumberyDude commented 1 year ago

проблема не решена

Какая? Вроде там все версии обновлены

ArtemMakhaydinov commented 1 year ago

281 (review)

проблема не решена

@TorinAsakura В более поздних версиях @oryd/keto-client и @ory/keto-client API поменялся

TorinAsakura commented 1 year ago

@ArtemMakhaydinov Ну так и чего? А обновление зависимостей, по-твоему, это просто изменение версий в пекедже?

ArtemMakhaydinov commented 1 year ago

@TorinAsakura Женя сказал в код не лезть. Полез разбираться.

ArtemMakhaydinov commented 1 year ago

@TorinAsakura установил @ory/keto-client: 0.0.0-alpha.58 вместо @oryd/keto-client: 0.0.0-alpha.46. В ory используется axios вместо request, у него схема респонса выглядит вот так:

{
  data: {},
  status: 200,
  statusText: 'OK',
  headers: {},
  config: {},
  request: {}
}

Поменял body на data - собирается, чекается, тестится. Для подъема на следующую версию 0.6 и выше полагаю надо переписать keto-access-control.guard.ts, т.к. модуля EnginesApi в новых версиях уже нет. Пока не понимаю как, но могу покопаться.

TorinAsakura commented 1 year ago

@ArtemMakhaydinov То есть не понимаешь как? Не понимаешь как код писать или что?

ArtemMakhaydinov commented 1 year ago

@TorinAsakura я не понимаю что именно должно получиться на выходе. Этот модуль куда-то встраивается и предоставляет там какие-то сервисы. Для этого создается объект из класса на 1К строк, предоставляемого апишкой старой версии keto. Что именно из этого ожидается потребителем сервиса я не знаю, в новых версиях keto структура API другая, а у @atls/nestjs-keto отсутствует документация.

TorinAsakura commented 1 year ago

@TorinAsakura я не понимаю что именно должно получиться на выходе. Этот модуль куда-то встраивается и предоставляет там какие-то сервисы. Для этого создается объект из класса на 1К строк, предоставляемого апишкой старой версии keto. Что именно из этого ожидается потребителем сервиса я не знаю, в новых версиях keto структура API другая, а у @atls/nestjs-keto отсутствует документация.

Оставь эту задачу. Рановато тебе пока туда нырять.

ArtemMakhaydinov commented 1 year ago

@TorinAsakura в виде компромисса ты мог бы объяснить что требуется, а я попытаюсь реализовать. Мне пока в любом случае есть чем заняться.

TorinAsakura commented 1 year ago

@TorinAsakura в виде компромисса ты мог бы объяснить что требуется, а я попытаюсь реализовать. Мне пока в любом случае есть чем заняться.

Тебе надо взять наш модуль для нестжс Поднять дефолтный инстанс кето и протестировать его работу Так понятней?

ArtemMakhaydinov commented 1 year ago

@TorinAsakura все понятно, кроме тестирования. Я использовал Jest, но год назад и не на модулях NestJS. Есть примеры использования?

TorinAsakura commented 1 year ago

@ArtemMakhaydinov Тебе дока неста не помогла или что? Интеграционные тесты есть в других пакетах