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

feat(core, react): Supports dynamic import for activities, and delays transition effects while loading an activity or waiting for a loader response #542

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

tonyfromundefined
Copy link
Member

@tonyfromundefined tonyfromundefined commented Dec 3, 2024

New Event (Core)

  • Provide PausedEvent and ResumedEvent to delay the effect of transitions.

New Interface (React)

import { lazy } from @stackflow/react/future”;

stackflow({
  // ...
  components: {
    MyActivity: lazy(() => import(../activities/MyActivity”)),
  },
});

(Note) This API is only supported by the Future API with Loader implementation

  • If the Activity is declared as lazy(() => import(“...”)), emit a PausedEvent before importing the Activity and a ResumedEvent if the Activity is successfully imported or an error occurs.
  • If the Activity has a loader declared, it raises a PausedEvent before executing the loader and a ResumedEvent when the loader state resolves.

Why does it have to be of the form lazy(() => import(“...”))?

Other candidates considered

// Plan A
stackflow({
  // ...
  components: {
    MyActivity: React.lazy(...),
  },
});

// Plan B
stackflow({
  // ...
  components: {
    MyActivity: () => import(...),
  },
});
  • For plan A, we have an issue where we don't know if it's wrapped in React.lazy or not. To find out, we'd have to float it in the React environment, which we decided was an unnecessary overhead.
  • Plan B also had the unnecessary overhead of having to run the function to evaluate whether the passed component or function returns a Promise or a React.ReactNode.

Therefore, I introduced a new construct lazy() that allows to statically evaluate whether the currently passed argument requires lazy loading or not.

Copy link

changeset-bot bot commented Dec 3, 2024

🦋 Changeset detected

Latest commit: 761b26d

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

This PR includes changesets to release 3 packages
Name Type
@stackflow/plugin-basic-ui Minor
@stackflow/react Minor
@stackflow/core Minor

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

Copy link

vercel bot commented Dec 3, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
stackflow-docs ✅ Ready (Inspect) Visit Preview Dec 26, 2024 4:56pm

@tonyfromundefined tonyfromundefined changed the title WIP(core, react): Delay transition effects when using React.lazy() to fetch an activity or when the loader is currently pending state. feat(core, react): Delay transition effects when using React.lazy() to fetch an activity or when the loader is currently pending state. Dec 10, 2024
@tonyfromundefined tonyfromundefined marked this pull request as ready for review December 10, 2024 09:30
@tonyfromundefined tonyfromundefined changed the title feat(core, react): Delay transition effects when using React.lazy() to fetch an activity or when the loader is currently pending state. feat(core, react): Supports dynamic import for activities, and delays transition effects while loading an activity or waiting for a loader response Dec 10, 2024
@tonyfromundefined
Copy link
Member Author

(참고) load -> lazy로 변경했습니다. cc. @2woongjae

Copy link
Member

@orionmiz orionmiz left a comment

Choose a reason for hiding this comment

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

I'm really appreciating your works for the tricky issue.

Here's a question:
Is PausedEvent and ResumedEvent allowed to be made by the 3rd-party users working with the plugins other than the integrations?

If so, please consider adding the hook interfaces for the plugin. (onBeforePaused, ...)

If not, hide those as internal events and block the event creation from calling dispatchEvent.

core/src/aggregate.ts Outdated Show resolved Hide resolved
@tonyfromundefined
Copy link
Member Author

@orionmiz I just finished reflecting your review, please review again.

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.

2 participants