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

Revert "Feat: update app published time after clicking publish button" #8320

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

crazywoola
Copy link
Member

@crazywoola crazywoola commented Sep 12, 2024

Reverts #7801
Fixes #8304

Can you @vicoooo26 take a look at this and open a new PR again?

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. ☕️ typescript Pull request that update TypeScript code. labels Sep 12, 2024
@crazywoola crazywoola merged commit aa11659 into main Sep 12, 2024
6 checks passed
@crazywoola crazywoola deleted the revert-7801-feat/publish_time branch September 12, 2024 12:06
@vicoooo26
Copy link
Contributor

Sorry for the late response.

In the previous PR, a new state publishedTime was introduced into the AppPublisher component and used propspublishedAt as the initial value. This state will never get updated even props update. It's regarded as an anti-pattern in React.

After browsing the codebase I found that there are two components that import AppPublisher. In the App Configuration component, the props publishedAt comes from memo and its dependency will not update. In the App Workflow component, the props publishedAt comes from store and will update in onPublish function.

In the App component maybe we could replace useMemo with useState or update the appDetail dependency in onPublish function.

What do you think? @crazywoola

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:S This PR changes 10-29 lines, ignoring generated files. ☕️ typescript Pull request that update TypeScript code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

All apps show as unpublished
2 participants