atls / nestjs

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

Обновление grpc-playground #237

Closed sollidy closed 1 year ago

sollidy commented 1 year ago

С чем связан запрос на фичу?

Обновление зависимостей в репозитории grpc-playground

Расскажите как вы это себе видите

После обновления зависимостей (минорных или мажорных версий) следующие шаги должны пройти без ошибок:

Приложите пример реализаций

SlumberyDude commented 1 year ago

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

Пока что при запуске yarn натыкаюсь на ошибку

@apollo/protobufjs@npm:1.2.2 couldn't be built successfully

В логах по данной ошибке

Error [ERR_LOADER_CHAIN_INCOMPLETE]: "file:///Users/kolodinevgenij/projects/atls/nestjs/.pnp.loader.mjs 'resolve'" did not call the next hook in its chain and did not explicitly signal a short circuit. If this is intentional, include `shortCircuit: true` in the hook's return.

Как будто ошибка внутри .pnp.loader.mjs, но гугление позволяет понять, что проблема в версиях Node.Js / yarn Баг описан вот тут https://github.com/yarnpkg/berry/issues/4778

Похоже, что нужно либо снизить версию ноды до 16.16 или старше (сейчас у меня 19.0.1), либо обновить yarn.

В репозитории лежит билд yarn yarnPath: .yarn/releases/yarn-0.0.1-git.20221107.hash-d21b699.cjs, который используется при установке зависимостей yarn install с которым появляется ошибка. При этом в package.json указано "packageManager": "yarn@3.3.0".

Если вероломно поставить версию в пакете в 3.3.0 выполнив команду yarn set version 3.3.0, то установка зависимостей с yarn install проходит нормально, но вносятся изменения в файлы конфигураций yarn вроде yarn.lock .yarnrc.yml

В связи с вышесказанным есть вопросы:

Попробовал установить зависимости с node.js 16.16.0, все работает. 16.17.0 уже падает с той же ошибкой.

Последняя версия 20.5.0, значит ли это, что под нее? Тогда судя по всему придется обновить yarn, что так или иначе затронет весь репозиторий, а не только grpc-playground воркспейс.

grpc-playground зависит от других воркспейсов, я так понимаю их я не трогаю, а пытаюсь обновить версии всего остального в данном пакете.

SlumberyDude commented 1 year ago

Судя по схожей таске https://github.com/atls/common/issues/6 сидеть на старой ноде (16.16.0) не вариант.

В таком случае встает вопрос с обновлением yarn в репозитории.

sollidy commented 1 year ago

последняя наша версия ярна лежит здесь:

sollidy commented 1 year ago

обновление ярна командой:

TorinAsakura commented 1 year ago

@sollidy уже обновил…щас закину

SlumberyDude commented 1 year ago

Вижу апдейт. Солью изменения и буду продолжать.

SlumberyDude commented 1 year ago

Чтобы обновить одну из зависимостей get-port https://www.npmjs.com/package/get-port/v/7.0.0 до последней версии придется обновить typescript, вроде до мажорной 5.0.0 (там впервые встречаю такое использование ключевого слова type), иначе yarn check не проходит тайпчек модуля.

../../../.yarn/berry/cache/get-port-npm-7.0.0-72b8a92f99-9.zip/node_modules/get-port/index.d.ts
Error: Module '"node:net"' has no exported member 'type'.

Ругается конкретно на вот эту строчку из пакета

import {type ListenOptions} from 'node:net';

Кажется странным, что чекаются пакеты зависимостей, это ок?

Пока что остался на более ранней версии пакета get-port 6.1.2 с которой все чеки проходят.

Не совсем понимаю по peerDependencies.

И еще,

Там версия неста наверное не зайдет, я обновляю до самой свежей 10.1.3

Открыл PR https://github.com/atls/nestjs/pull/243

Чет все интеграционные тесты упали

sollidy commented 1 year ago

https://github.com/atls/hyperion/wiki/%D0%A7%D1%82%D0%BE-%D1%82%D0%B0%D0%BA%D0%BE%D0%B5-peerDependencies

SlumberyDude commented 1 year ago

Почитал про peerDependencies

Я правильно понимаю, что мне нужно полностью их прокачать до последних версий (какие возможно), как я это сделал в devDependencies?

Это будет требовать от родительского пакета (от клиента) использования именно последней версии неста или выше ^10.1.3

Не правильнее ли зафиксировать только мажорную, как это было сделано до обновления (была ^8.0.0), то есть поставить ^10.0.0?

TorinAsakura commented 1 year ago

@SlumberyDude Нет. Не правильно.

SlumberyDude commented 1 year ago

Нужно ли трогать модуль rxjs? Его версия 7.5.4 есть в resolutions в корневом package.json и вроде бы это не дает мне использовать в обновляемом модуле версию выше. Либо же фиксить resolutions, убирать из него rxjs например.

TorinAsakura commented 1 year ago

@SlumberyDude Ты всегда должен пробовать грейдиться, чтобы, как минимум, проверить на потенциальные проблемы совместимости

SlumberyDude commented 1 year ago

@TorinAsakura Если обновляю rxjs до последней 7.8.1 внутри grpc-playground, то все чеки и юниты проходят, но вебшторм указывает на несовпадение версий с той, которая есть в корневом resolutions.

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

Также до конца не понял ответ на вчерашний вопрос. Повышаю до упора peerDependency или нет?

SlumberyDude commented 1 year ago

Мысли по поводу провала тайпчека в PR

После удаления зависимости rxjs 7.5.4 из resolutions возникает ошибка типов в пакете dataloader. Связана с несовпадением версий rxjs.

@atls/nestjs-dataloader
├── @nestjs/common@8.0.5
│   └── rxjs@7.5.4
└── rxjs@7.5.2

Пытался поменять версию rxjs на 7.5.2 для @nestjs/common@8.0.5 с использованием packageExtensions, но безуспешно. При установке зависимостей получаю

rxjs: This rule seems redundant when applied on the original package; the extension may have been applied upstream.

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

Note: This field is made to add dependencies; if you need to rewrite existing ones, prefer the resolutions field.


Сейчас пытаюсь что-то сделать через selective dependency resolutions


Добавил селективный resolution для пакета nestjs-dataloader

"resolutions": {
"@atls/nestjs-dataloader/rxjs": "7.5.4",

Теперь разбираюсь с интеграционными тестами

SlumberyDude commented 1 year ago

Фикс интеграционных тестов

По закрытым PR видно, что интеграционные тесты падали с момента своего появления 20 февраля 2022 года https://github.com/atls/nestjs/pull/223/checks?check_run_id=5259703452 в тех же модулях


Гипотезы проблемы:

TorinAsakura commented 1 year ago

@SlumberyDude Это не отменяет наличия проблемы. Мне известно почему они падают, а тебе?

TorinAsakura commented 1 year ago

@SlumberyDude А никто и не говорил про подмену. Я же тебе на созвоне объяснял, что это делается для добавления отсутствующих и в качестве примера привёл частую проблему с отсутствием типов в некоторых зависимостях…

SlumberyDude commented 1 year ago

@TorinAsakura

Мне известно почему они падают, а тебе?

Пока что нет, сейчас копаю в сторону @atls/code-test-worker.

А никто и не говорил про подмену. Я же тебе на созвоне объяснял, что это делается для добавления отсутствующих и в качестве примера привёл частую проблему с отсутствием типов в некоторых зависимостях…

Тут я скорее исходил из твоего комментария к PR:

...это ровно тот случай, о котором я сегодня рассказывал, случай, когда необходимо использовать packageExtension, так как мы не можем позволить себе поставить самую последнюю версию rxjs из-за несовместимости окружения

Не воспринимай мои комментарии как упрек или ещё что-то. Я только записываю то, что понял и то до чего удалось докопаться, чтобы быть прозрачным и вы вовремя увидели, если я в рассуждениях сверну не туда.

SlumberyDude commented 1 year ago

На локале добавление sqlite3 в dev зависимости пакета typesense-typeorm решило проблему интеграционных тестов.

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

``` Nest can't resolve dependencies of the KratosExceptionFilter (?). Please make sure that the argument Object at index [0] is available in the KratosModule context. Potential solutions: - If Object is a provider, is it part of the current KratosModule? - If Object is exported from a separate @Module, is that module imported within KratosModule? @Module({ imports: [ /* the Module containing Object */ ] }) ```

Необходимо выяснить особенности запуска интеграционных тестов ярном.

SlumberyDude commented 1 year ago

Анализ интеграционных тестов в kratos

Цепочка инжектов зависимостей

KratosModule ---> KratosExceptionFilter --- ошибка тут  ---> KratosBrowserUrls ---> @Inject(KRATOS_MODULE_OPTIONS) private readonly options: KratosModuleOptions

Наличие последнего инжекта в данной цепочке вызывает ошибку.


Пофиксил.

Остался один нерабочий интеграционный тест на typesense-typeorm, но там что-то неясно, вчера же все проходило. На локале тоже возникала эта ошибка, прошла при повторном запуске.

SlumberyDude commented 1 year ago

Интеграционные тесты модуля typesense-typeorm

При чеке на гитхабе получаю ошибку

``` Error: expect(received).toBe(expected) // Object.is equality Expected: 1 Received: 2 at Object. (/home/runner/work/nestjs/nestjs/packages/typesense-typeorm/integration/typesense-typeorm.test.ts:72:30) at processTicksAndRejections (node:internal/process/task_queues:95:5) ```

Тест состоит из 2х кейсов, каждый из которых записывает объект в базу, затем ищет через typesense этот объект и проверяет, что нашелся один объект. То есть должен находить 1-1, тогда как в данном случае в ошибке выше видим, что нашлось 2 объекта во втором кейсе, т.е. результат 1-2

На локальном сервере тест проходит нестабильно.

Чаще всего оба кейса отрабатывают нормально, то есть результат 1-1 но встречаются различные комбинации, как 0-1, 1-2.


Имеется 2 проблемы

  1. После первого кейса база не очищается и казалось бы успешный случай 1-1 соответствует тому, что второй кейс находит объект записанный в 1 кейсе, а измененный объект в своем кейсе не видит (он не был изменен).

  2. При поиске объекта в БД с помощью typesense по каким-то причинам изменения в базе не успевают вступить в силу и поиск показывает неверный результат (не находит только что созданный/измененный объект). При этом данная ошибка носит вероятностный характер.


    Предлагаемые способы исправления

    • Исправить проверку expect(result.found).toBe(1) во втором кейсе на expect(result.found).toBe(2) и не очищать базу между кейсами.
    • Очистить базу между кейсами
  3. Возможно выставление задержки между изменением в базе и поиском в typesense. В идеале разобраться, почему казалось бы синхронный код (изменение базы через await repository.save(entity)) не успевает внести изменения в базу к следующей инструкции.

Почему typesense не поспевает за обновлением в базе.

В модуле typesense-typeorm создается TypeOrmListenersBuilder, который создает слушателей. Последние отслеживают действия над репозиторием БД и вносят соответствующие обновления в typesense.

```js afterInsert(event: InsertEvent) { this.mapper.insert(event.entity) } afterUpdate(event: UpdateEvent) { this.mapper.update(event.entity) } ```

Видим, что обновление typesense вызывается асинхронно после действий с БД не дожидаясь результата.

Поэтому тесты не проходят, typesense не успевает обновиться вслед за БД.

Считаю, что необходимо добавить дилей в тест между обновлением сущности в БД и поиском, чтобы typesense успевал синхронизировать изменения. 5 ms хватает