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: Support <Link /> in Future API #509

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

Conversation

tonyfromundefined
Copy link
Member

@tonyfromundefined tonyfromundefined commented Aug 21, 2024

내용

  • 기존의 <Link /> 컴포넌트는 @stackflow/plugin-history-sync에 의존하고 있었어요. (import를 함)
    • 이번에 뜯게되면서 이 부분에서 개선점을 발견해서 개선을 하고 싶었어요. (의존도를 낮추자)
  • 사실 <Link /> 컴포넌트는 @stackflow/plugin-history-sync에 직접적으로 의존하지 않는 형태에요.
    • @stackflow/plugin-history-sync가 깔려있으면, href="..."를 셋팅하고, 아니면 href를 없애면 돼요.
    • "이걸 어떻게 구현할까?" 가 고민의 핵심이에요.
  • config를 확장하는 인터페이스를 고안해서 구현해보았어요. 전반적인 설계도가 나온건 아니지만 PoC 느낌으로 config를 확장하는 기능을 만들었어요.
    • 이번에 Declaration Merging을 적극적으로 도입한 김에 Declaration Merging으로 유명한 Fastify Plugin 인터페이스의 app.decorate() 방식을 참고했어요.

Copy link

changeset-bot bot commented Aug 21, 2024

🦋 Changeset detected

Latest commit: 99d4e92

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

This PR includes changesets to release 7 packages
Name Type
@stackflow/plugin-history-sync Minor
@stackflow/react Minor
@stackflow/link Minor
@stackflow/config Minor
@stackflow/demo Minor
@stackflow/docs Patch
@stackflow/plugin-preload Patch

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 Aug 21, 2024

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

Name Status Preview Updated (UTC)
stackflow-docs ✅ Ready (Inspect) Visit Preview Sep 23, 2024 10:12am

Comment on lines 2 to 5
import type { ConfigDefinition } from "./ConfigDefinition";

export type Config<T extends ActivityDefinition<string>> = {
activities: T[];
transitionDuration: number;
initialActivity?: () => T["name"];
};
export interface Config<T extends ActivityDefinition<string>>
extends ConfigDefinition<T> {}
Copy link
Member Author

Choose a reason for hiding this comment

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

ConfigDefinition과 Config를 분리합니다

extensions/plugin-history-sync/src/historySyncPlugin.tsx Outdated Show resolved Hide resolved
extensions/plugin-history-sync/src/historySyncPlugin.tsx Outdated Show resolved Hide resolved
integrations/react/src/future/Link.tsx Outdated Show resolved Hide resolved
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.

고생하셨습니다~
하나 궁금한 점이 있는데 preload 기능은 어떻게 대체되는건가요?

demo/src/components/ArticleCard.tsx Outdated Show resolved Hide resolved
extensions/plugin-history-sync/src/historySyncPlugin.tsx Outdated Show resolved Hide resolved
extensions/plugin-history-sync/src/historySyncPlugin.tsx Outdated Show resolved Hide resolved
@tonyfromundefined
Copy link
Member Author

tonyfromundefined commented Sep 23, 2024

고생하셨습니다~ 하나 궁금한 점이 있는데 preload 기능은 어떻게 대체되는건가요?

기본 동작이 너무 복잡해보여서 제거했어요. 추후에 <Link preload=...> 와 같이 props로 옵션을 제공하는 편이 좋을거같아용. 옵션 자체에 대한 Policy도 잘 정의해놓구요.

참고:

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