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.

Sign up for GitHub

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

Jump to bottom

TreeView: Don't use async/await #4529

Closed
julien opened this issue Jan 3, 2022 · 9 comments · Fixed by #4532
Closed

TreeView: Don't use async/await #4529

julien opened this issue Jan 3, 2022 · 9 comments · Fixed by #4532
Labels
type: bug Issues reporting that Component is not doing what should be done
Milestone
LPS-144180 - Bi-w...

Comments

@julien
Copy link
Contributor

julien commented Jan 3, 2022

In the TreeViewItem component we're using async/await, but
since we can't use that in liferay-portal, we should avoid it.

This (imo) is another good reason why we should move Clay inside DXP (see #3849), because it would
also make us realize (or remember) these kind of things in advance instead of living in our own "Clay world"
@julien julien added the type: bug Issues reporting that Component is not doing what should be done label Jan 3, 2022
julien pushed a commit to julien/clay that referenced this issue Jan 4, 2022
fix(@clayui/core): avoid async/await in TreeViewItem
78f8585 
Since we can't use async/await in DXP, we should also avoid using it in
our components

Fixes liferay#4529
Depends on liferay#4531
@julien julien mentioned this issue Jan 4, 2022
Merged
@julien julien mentioned this issue Jan 4, 2022
Closed
@bryceosterhaus
Copy link
Member

@julien could you provide a bit of context as for "why" we can't use async/await in portal?

@bryceosterhaus
Copy link
Member

I would expect it should be okay to use in liferay-portal, unless I forgot about something, which is pretty likely.
Screen Shot 2022-01-06 at 3 06 27 PM

@julien
Copy link
Contributor Author

julien commented Jan 11, 2022

Hey @bryceosterhaus thanks for looking into this. I'll post more details in a bit, but basically I was getting errors about "regenerator runtime". Stay tuned for more.

@julien
Copy link
Contributor Author

julien commented Jan 11, 2022

I just tested again, and you are right: in DXP if you use async/await with no problem.

The problem comes from "here" (the way we build/transpile in Clay), when using this component and deploying the module I'm seeing this:

And here's a more "detailed" view of line 158

So maybe we're missing a babel plugin and that's just it. What do you think @bryceosterhaus?

@julien
Copy link
Contributor Author

julien commented Jan 11, 2022

@bryceosterhaus this is what I tried

yarn add --dev -W @babel/plugin-transform-runtime
yarn add -W @babel/runtime

Add '@babel/plugin-transform-runtime' to the "plugins" in our babel.config.js file.
Rebuilt the clayui/core package, and I don't see anymore errors.

If that sounds good, I can send a PR with all that + re-adding all the "async/await" stuff I removed, because I actually preferred it 😂.

@bryceosterhaus
Copy link
Member

Ah I see, yeah my guess would be that we had something in Clay that is transpiling async/await stuff because at the time of adding it, we didn't support it.

We might consider auditing our build a bit to make sure we aren't over-doing it when it comes to transpiling for older browsers.

julien pushed a commit to julien/liferay-portal that referenced this issue Jan 12, 2022
LPS-144181 Update Clay dependencies
6b8b68f 
 ## [3.44.2](liferay/clay@v3.44.1...v3.44.2) (2022-01-12)

 ### Bug Fixes

 -   **@clayui/css:** Cadmin Nav adds `background-color` to `.active` pseudo element ([865b037](liferay/clay@865b037)), closes [liferay#4562](liferay/clay#4562)
 -   **@clayui/css:** Mixins `clay-button-variant` removes unnecessary `setter()`'s ([3709d7f](liferay/clay@3709d7f)), closes [liferay#4550](liferay/clay#4550)

 ## [3.44.1](liferay/clay@v3.44.0...v3.44.1) (2022-01-11)

 ### Bug Fixes

 -   **@clayui/core:** remove the context property from DndProvider ([4faf0ea](liferay/clay@4faf0ea))
 -   **@clayui/css:** Mixins `clay-card-variant` allow styling before and after psuedo elements ([69f1d38](liferay/clay@69f1d38)), closes [liferay#4554](liferay/clay#4554)

 # [3.44.0](liferay/clay@v3.43.1...v3.44.0) (2022-01-10)

 ### Bug Fixes

 -   **@clayui/css:** Cadmin Input Groups `input-group-sm` missing mixin declaration ([d7027be](liferay/clay@d7027be)), closes [liferay#4537](liferay/clay#4537)
 -   **@clayui/css:** Cards `form-check-card` remove duplicate hover state style ([17ea640](liferay/clay@17ea640))
 -   **@clayui/css:** Mixins Cards check if parameter is map to avoid must be a map error ([2174587](liferay/clay@2174587))
 -   **@clayui/css:** Mixins Custom Forms remove `setter()`, no longer needed ([4cb30ce](liferay/clay@4cb30ce))

 ### Features

 -   **@clayui/css:** Cadmin Nav adds `nav-divider` and `nav-divider-end` ([66ba6ce](liferay/clay@66ba6ce))
 -   **@clayui/css:** Cards `form-check-card` checkbox/radio input should be hidden unless position utilities are used ([3120797](liferay/clay@3120797)), closes [liferay#4544](liferay/clay#4544)
 -   **@clayui/css:** Cards `form-check-card` convert to Clay mixin pattern ([5b4424b](liferay/clay@5b4424b))
 -   **@clayui/css:** Mixin `clay-card-variant` make `form-check-card` and `form-check-input` customizable ([c6a3a6f](liferay/clay@c6a3a6f))
 -   **@clayui/css:** Mixins `clay-custom-control-input-variant` add option to customize card ([87682d2](liferay/clay@87682d2))
 -   **@clayui/css:** Mixins `clay-navbar-variant` adds option to customize `nav-divider` and `nav-divider-end` ([e58c23e](liferay/clay@e58c23e))
 -   **@clayui/css:** Mixins Card adds `clay-form-check-card-variant` ([a834502](liferay/clay@a834502))
 -   **@clayui/css:** Nav adds `nav-divider` and `nav-divider-end` ([600a379](liferay/clay@600a379))
 -   **@clayui/css:** SVG Icons adds date-time ([4bf8d4c](liferay/clay@4bf8d4c))
 -   **@clayui/popover:** add a closeOnClickOutside prop ([bd722a9](liferay/clay@bd722a9)), closes [liferay#4536](liferay/clay#4536)

 ## [3.43.1](liferay/clay@v3.43.0...v3.43.1) (2022-01-04)

-   **@clayui/core:** avoid async/await in TreeViewItem ([78f8585](liferay/clay@78f8585)), closes [liferay#4529](liferay/clay#4529) [liferay#4531](liferay/clay#4531)
-   **@clayui/core:** don't export TreeView as UNSAFE_TreeView ([fc54a6e](liferay/clay@fc54a6e)), closes [liferay#4528](liferay/clay#4528) [liferay#4531](liferay/clay#4531) [liferay#4532](liferay/clay#4532)
@julien julien mentioned this issue Jan 12, 2022
Closed
Closed
brianchandotcom pushed a commit to brianchandotcom/liferay-portal that referenced this issue Jan 12, 2022
@brianchandotcom
LPS-144181 Update Clay dependencies
7fd95b6 
 ## [3.44.2](liferay/clay@v3.44.1...v3.44.2) (2022-01-12)

 ### Bug Fixes

 -   **@clayui/css:** Cadmin Nav adds `background-color` to `.active` pseudo element ([865b037](liferay/clay@865b037)), closes [#4562](liferay/clay#4562)
 -   **@clayui/css:** Mixins `clay-button-variant` removes unnecessary `setter()`'s ([3709d7f](liferay/clay@3709d7f)), closes [#4550](liferay/clay#4550)

 ## [3.44.1](liferay/clay@v3.44.0...v3.44.1) (2022-01-11)

 ### Bug Fixes

 -   **@clayui/core:** remove the context property from DndProvider ([4faf0ea](liferay/clay@4faf0ea))
 -   **@clayui/css:** Mixins `clay-card-variant` allow styling before and after psuedo elements ([69f1d38](liferay/clay@69f1d38)), closes [#4554](liferay/clay#4554)

 # [3.44.0](liferay/clay@v3.43.1...v3.44.0) (2022-01-10)

 ### Bug Fixes

 -   **@clayui/css:** Cadmin Input Groups `input-group-sm` missing mixin declaration ([d7027be](liferay/clay@d7027be)), closes [#4537](liferay/clay#4537)
 -   **@clayui/css:** Cards `form-check-card` remove duplicate hover state style ([17ea640](liferay/clay@17ea640))
 -   **@clayui/css:** Mixins Cards check if parameter is map to avoid must be a map error ([2174587](liferay/clay@2174587))
 -   **@clayui/css:** Mixins Custom Forms remove `setter()`, no longer needed ([4cb30ce](liferay/clay@4cb30ce))

 ### Features

 -   **@clayui/css:** Cadmin Nav adds `nav-divider` and `nav-divider-end` ([66ba6ce](liferay/clay@66ba6ce))
 -   **@clayui/css:** Cards `form-check-card` checkbox/radio input should be hidden unless position utilities are used ([3120797](liferay/clay@3120797)), closes [#4544](liferay/clay#4544)
 -   **@clayui/css:** Cards `form-check-card` convert to Clay mixin pattern ([5b4424b](liferay/clay@5b4424b))
 -   **@clayui/css:** Mixin `clay-card-variant` make `form-check-card` and `form-check-input` customizable ([c6a3a6f](liferay/clay@c6a3a6f))
 -   **@clayui/css:** Mixins `clay-custom-control-input-variant` add option to customize card ([87682d2](liferay/clay@87682d2))
 -   **@clayui/css:** Mixins `clay-navbar-variant` adds option to customize `nav-divider` and `nav-divider-end` ([e58c23e](liferay/clay@e58c23e))
 -   **@clayui/css:** Mixins Card adds `clay-form-check-card-variant` ([a834502](liferay/clay@a834502))
 -   **@clayui/css:** Nav adds `nav-divider` and `nav-divider-end` ([600a379](liferay/clay@600a379))
 -   **@clayui/css:** SVG Icons adds date-time ([4bf8d4c](liferay/clay@4bf8d4c))
 -   **@clayui/popover:** add a closeOnClickOutside prop ([bd722a9](liferay/clay@bd722a9)), closes [#4536](liferay/clay#4536)

 ## [3.43.1](liferay/clay@v3.43.0...v3.43.1) (2022-01-04)

-   **@clayui/core:** avoid async/await in TreeViewItem ([78f8585](liferay/clay@78f8585)), closes [#4529](liferay/clay#4529) [#4531](liferay/clay#4531)
-   **@clayui/core:** don't export TreeView as UNSAFE_TreeView ([fc54a6e](liferay/clay@fc54a6e)), closes [#4528](liferay/clay#4528) [#4531](liferay/clay#4531) [#4532](liferay/clay#4532)
@github-actions
Copy link

This issue has been merged and will be released in DXP at https://issues.liferay.com/browse/LPS-144180

@matuzalemsteles
Copy link
Member

Oh, this is really bad going back to using Promise instead of async/await. I thought we were already supporting this in DXP. Actually, Clay is already transpiling to work in older browsers but polyfill is not being added in DXP, we can also stop transpiling but this will only work in newer browsers. Since we don't support IE11.

@julien
Copy link
Contributor Author

julien commented Jan 25, 2022

Hey @matuzalemsteles, did you read my previous comments?

Oh, this is really bad going back to using Promise instead of async/await

Well it's not that bad to be honest, it's just a matter of using the correct babel plugins (in my opinion).
async/await works in DXP, what doesn't work is consuming Clay's transpiled async/await code in DXP.

I thought we were already supporting this in DXP.

Have you ever tried it though? (Using a Clay component that uses async/await and seeing it work in DXP)?

we can also stop transpiling but this will only work in newer browsers. Since we don't support IE11.

For me that wouldn't be an issue, but we need to make sure that's OK in DXP 7.3 (where Clay 3.x is used).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type: bug Issues reporting that Component is not doing what should be done
Projects
None yet
Milestone
LPS-144180 - Bi-weekly Clay release J...
3 participants
@julien @bryceosterhaus @matuzalemsteles