Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

plasma-web/plasma-b2c: Добавлена сборка без styled-components #942

Merged
merged 3 commits into from Feb 12, 2024

Conversation

Yeti-or
Copy link
Contributor

@Yeti-or Yeti-or commented Dec 21, 2023

Добавили возможность использовать plasma-web & plasma-b2c без styled-components

Пример использования:

import { TextArea } from '@salutejs/plasma-web/css';

What/why changed

Добавлены скрипты которые генерируют нужную структуру из тех компонентов которые есть в plasma-new-hope
дальше подрубаемся rollup и генерируем стили

📦 Published PR as canary version: Canary Versions

✨ Test out this PR locally via:

npm install @salutejs/plasma-b2c@1.280.1-canary.942.7800621499.0
npm install @salutejs/plasma-web@1.280.1-canary.942.7800621499.0
# or 
yarn add @salutejs/plasma-b2c@1.280.1-canary.942.7800621499.0
yarn add @salutejs/plasma-web@1.280.1-canary.942.7800621499.0

@@ -11,4 +10,4 @@ const SpinnerComponent = component(mergedConfig) as React.FunctionComponent<Spin
/**
* Компонент для отображения индикатора загрузки.
*/
export const Spinner = styled(SpinnerComponent)``;
export const Spinner = SpinnerComponent;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@neretin-trike напомни зачем тут это нужно было

Copy link
Collaborator

Choose a reason for hiding this comment

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

ну он в styled был обёрнут (и раньше так было, до перевода на новую архитектуру) для того, чтобы не сломалось у тех, кто возможно стилизовал Spinner через styled-component и обращался к нему как классу, типа:

const StyledWrapper = styled.div`
   ${Spinner} {
      background: blue
   }
` 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Блин чёт я не понимаю тогда как это разрулить :/

Copy link
Contributor

Choose a reason for hiding this comment

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

Я это проверил, и это работает только через styled от SC. Так что на линарии этот функционал не будет работать. Пруф
CC: @Yeti-or @neretin-trike

@Yeti-or Yeti-or marked this pull request as ready for review December 21, 2023 20:58
@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-942/

Copy link
Contributor

⚡ Component performance testing

Result: 🟢 OK

@Salute-Eva
Copy link
Contributor

@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-942/

@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-942/

@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-942/

@Salute-Eva
Copy link
Contributor

@@ -30,3 +30,6 @@ done;
# plasma-new-hope/styled-components => plasma-new-hope
perl -p -i -e "s/\/styled-components//g" $files

#TODO: #1024 удалить обертку styled в спиннере
perl -p -i -e "s/import styled from 'styled-components';//g" src-css/components/Spinner/Spinner.tsx
perl -p -i -e "s/styled\(SpinnerComponent\)\`\`/SpinnerComponent/g" src-css/components/Spinner/Spinner.tsx
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ааа ты тут просто удаляешь обертку и в линарии идет FunctionComponent

Copy link
Contributor

Choose a reason for hiding this comment

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

В линарии идет сразу компонент, без styled, ибо он запрещен в рантайме и это в принципе бессмысленно.

Copy link
Collaborator

Choose a reason for hiding this comment

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

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

const StyledWrapper = styled.div`
   ${Spinner} {
      background: blue
   }
` 

Copy link
Contributor

Choose a reason for hiding this comment

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

Думаю, что да, но зачем нам это?

Copy link
Collaborator

Choose a reason for hiding this comment

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

ну это нужно было проверить, чтобы убедиться, что у пользователей, которые вот таким образом спиннер стилизуют, поменяв путь с plasma-b2c на plasma-b2c/css, не сломается сборка

Copy link
Collaborator

Choose a reason for hiding this comment

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

А зачем ребятам на sc импортировать /css сборку? Мы же сделали этот фикс, чтобы спиннер был обернут в styled из sc, чтобы на сборке на стайледах все было по-прежнему.

ну когда мы обсуждали это, то как раз пришли к такому мнению, что те, кто будут постепенно переезжать с styled-compoennts на линарию, будут иметь и те, и другие импорты (и зависимости)

Copy link
Collaborator

Choose a reason for hiding this comment

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

А сборку /css будут юзать те, у кого нет sc (хотя таковых на данный момент нет). И там я не вижу смысла оборачивать именно Спиннер в styled из линарии, ибо зачем?

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

Copy link
Contributor

Choose a reason for hiding this comment

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

А, ну эти проблемы точно будут, и потребуют ручного устранения, ибо конструкция
${Spinner} { background: blue }
ожидает только styled-подобный компонент из SC, и ни стайлед от линарии, ни обычный react.element как минимум не пройдут проверку типов, а если и пройдут, то работать это не будет.

Copy link
Collaborator

Choose a reason for hiding this comment

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

а такая конструкция валидна только для styled.tagname компонент получается? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Это вполне валидный паттерн, как для SC так и для линарии. https://github.com/callstack/linaria/blob/master/docs/BASICS.md#referring-to-another-component-or-class-name

@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-942/

@Salute-Eva
Copy link
Contributor

Copy link
Contributor Author

@Yeti-or Yeti-or left a comment

Choose a reason for hiding this comment

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

Давайте вольем текущее состояние а дальше будем править уже по мере проблем

touch src-css/index.ts
touch src-css/index.d.ts
for component in $components; do
if [[ "$component" != "Switch" ]]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

Решили пока оставить так, в этом issue нужно будет потом починить

@shuga2704 shuga2704 added this pull request to the merge queue Feb 12, 2024
Merged via the queue into dev with commit fb1a112 Feb 12, 2024
24 of 25 checks passed
@shuga2704 shuga2704 deleted the yeti-or.add-css-build branch February 12, 2024 08:15
@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/plasma-b2c@1.281.1-dev.0, @salutejs/plasma-temple-docs@0.219.1-dev.0, @salutejs/plasma-ui-docs@0.268.1-dev.0, @salutejs/plasma-web-docs@0.235.1-dev.0, @salutejs/plasma-web@1.281.1-dev.0, @salutejs/plasma-website@0.243.1-dev.0 🚀

@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/plasma-b2c@1.281.1-dev.0, @salutejs/plasma-temple-docs@0.219.1-dev.0, @salutejs/plasma-ui-docs@0.268.1-dev.0, @salutejs/plasma-web-docs@0.235.1-dev.0, @salutejs/plasma-web@1.281.1-dev.0, @salutejs/plasma-website@0.243.1-dev.0 🚀

1 similar comment
@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/plasma-b2c@1.281.1-dev.0, @salutejs/plasma-temple-docs@0.219.1-dev.0, @salutejs/plasma-ui-docs@0.268.1-dev.0, @salutejs/plasma-web-docs@0.235.1-dev.0, @salutejs/plasma-web@1.281.1-dev.0, @salutejs/plasma-website@0.243.1-dev.0 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants