Skip to content

feat/lottie#2035

Open
hextion wants to merge 9 commits into
masterfrom
feat/lottie
Open

feat/lottie#2035
hextion wants to merge 9 commits into
masterfrom
feat/lottie

Conversation

@hextion
Copy link
Copy Markdown
Collaborator

@hextion hextion commented Feb 2, 2026

No description provided.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Feb 2, 2026

🦋 Changeset detected

Latest commit: 063cd8a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@alfalab/core-components Minor
@alfalab/core-components-lottie Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 2, 2026

Bundle size report

Entry point Size (minified)
@alfalab/core-components-lottie/index.js 171.8 (+171.80 KB ❌)

@coveralls
Copy link
Copy Markdown

coveralls commented Feb 2, 2026

Coverage Report for CI Build 24834735888

Coverage remained the same at 81.985%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 11606
Covered Lines: 9596
Line Coverage: 82.68%
Relevant Branches: 2177
Covered Branches: 1704
Branch Coverage: 78.27%
Branches in Coverage %: Yes
Coverage Strength: 230.44 hits per line

💛 - Coveralls

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 2, 2026

Demo build

https://core-ds.github.io/core-components/2035

@hextion hextion force-pushed the feat/lottie branch 4 times, most recently from 739f76d to 9dcb40e Compare February 6, 2026 07:17
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 6, 2026

Demo build (default)

https://core-ds.github.io/core-components/2035

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 6, 2026

Demo build (alfasans)

https://core-ds.github.io/core-components/2035-alfasans

@hextion hextion force-pushed the feat/lottie branch 3 times, most recently from 1ad4b85 to 87fc4f0 Compare February 13, 2026 08:32
@hextion hextion force-pushed the feat/lottie branch 2 times, most recently from 53bfe40 to 2a0ca51 Compare March 2, 2026 07:04
@hextion hextion marked this pull request as ready for review March 2, 2026 09:10
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 2, 2026

Snapshot release (default)

Successfully released the following packages:

  1. @alfalab/core-components-lottie@1.0.0-1960cb50f1bb912c4c88a4e9041989958f542451

  2. @alfalab/core-components@50.6.0-1960cb50f1bb912c4c88a4e9041989958f542451

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 2, 2026

Snapshot release (alfasans)

Successfully released the following packages:

  1. @alfalab/core-components-lottie@1.0.0-alfasans-1960cb50f1bb912c4c88a4e9041989958f542451

  2. @alfalab/core-components@50.6.0-alfasans-1960cb50f1bb912c4c88a4e9041989958f542451

@SiebenSieben SiebenSieben requested a review from Oladii March 13, 2026 07:28
SiebenSieben

This comment was marked as duplicate.

SiebenSieben

This comment was marked as duplicate.

SiebenSieben

This comment was marked as duplicate.

SiebenSieben

This comment was marked as duplicate.

SiebenSieben

This comment was marked as duplicate.

SiebenSieben

This comment was marked as duplicate.

SiebenSieben

This comment was marked as duplicate.

SiebenSieben

This comment was marked as duplicate.

SiebenSieben

This comment was marked as duplicate.

Copy link
Copy Markdown
Contributor

@SiebenSieben SiebenSieben left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Отсутствуют unit-тесты. В библиотеке это обязательное требование для каждого компонента — нужен Component.test.tsx. Без тестов сложно гарантировать корректность поведения при будущих изменениях.

export const lottie: Story = {
name: 'Lottie',
render: () => {
return <Lottie animation={{ path: 'http://localhost:9009/twitter-heart.json' }} />;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Хардкод localhost — в задеплоенном Storybook это сломается. Нужно использовать относительный путь, аналогично тому как сделано в description.mdx (./twitter-heart.json).

direction={direction}
speed={1}
iterations={0}
direction={direction}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Дублирующийся проп direction={direction}. JSX не выдаст ошибку компиляции в MDX, но второй проп перезаписывает первый — нужно убрать одно из вхождений.

[events, reset],
);
useImperativeHandle(playCountRef, () => animation?.playCount ?? 0, [animation]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useImperativeHandle предназначен для кастомизации хендла, прокидываемого наружу через forwardRef. Использование его на внутреннем playCountRef нестандартно и вводит в заблуждение. Достаточно обычного эффекта:

useEffect(() => {
    playCountRef.current = animation?.playCount ?? 0;
}, [animation]);

Comment thread .eslintrc.cjs
'max-params': 'off',
'react-hooks/exhaustive-deps':
lottiePkg && isSamePath(process.cwd(), lottiePkg.dir)
? ['warn', { additionalHooks: ['useLayoutEffect_SAFE_FOR_SSR'].join('|') }]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Два вопроса по этому изменению:

  1. useLayoutEffect_SAFE_FOR_SSR из @alfalab/hooks используется во всём репозитории, не только в lottie. Если хук нужно добавить в additionalHooks — логичнее сделать это глобально для всех пакетов, а не только при isSamePath(..., lottiePkg.dir).

  2. ["useLayoutEffect_SAFE_FOR_SSR"].join("|") возвращает просто "useLayoutEffect_SAFE_FOR_SSR".join("|") на массиве из одного элемента избыточен.

animationItem.destroy();
// eslint-disable-next-line no-underscore-dangle
animationItem._cbs = [];
setAnimation(null);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Прямая запись в приватное поле _cbs обходит публичный API lottie-web. При обновлении библиотеки это поле может исчезнуть или изменить структуру, и баг будет трудно отловить. Если это необходимо для предотвращения утечек памяти — стоит добавить подробный комментарий, почему это нельзя сделать через animationItem.destroy() или другой публичный способ.

Comment thread packages/lottie/package.json Outdated
"tslib": "^2.4.0"
},
"peerDependencies": {
"react": "^16.9.0 || ^17.0.1 || ^18.0.0",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Все остальные пакеты в репозитории уже добавили поддержку React 19. Нужно привести в соответствие:

"react": "^16.9.0 || ^17.0.1 || ^18.0.0 || ^19.0.0",
"react-dom": "^16.9.0 || ^17.0.1 || ^18.0.0 || ^19.0.0"

</div>
);
},
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не хватает displayName — это стандарт библиотеки (см. Button, Checkbox и др.):

Lottie.displayName = "Lottie";

* Дополнительный класс
*/
className?: string;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В библиотеке принято поддерживать dataTestId во всех компонентах для использования в автотестах. Нужно добавить проп и прокинуть через data-test-id на корневой элемент.

@hextion hextion force-pushed the feat/lottie branch 3 times, most recently from b31db82 to 8a6e27c Compare March 22, 2026 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants