isom-1659 internal links for folders #865
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Datadog ReportBranch report: ✅ 0 Failed, 184 Passed, 34 Skipped, 43.04s Total Time |
…ngohjw/isom-1659-internal-links-for-folders
| )} | ||
| </> | ||
| <ResourceSelector | ||
| queryFn={trpc.resource.getChildrenOf.useInfiniteQuery} |
There was a problem hiding this comment.
nit: to be honest, this feels like a code-smell. this works for now because they have relatively similar interfaces but we tie it very closely to the actual function too (the type of queryFn is specified as those 2 queries). i worry that if change is needed to either of the 2 functions, this component might not be as easily reusable down the line.
something we could consider is shifting the subcomponents out and favouring composition so that, for example, 1 component purely displays another and another triggers the refetch. another alternative we could do is specify the type ourselves + specify the refetch etc as extra props so we don't depend so strictly on teh trpc type!
There was a problem hiding this comment.
i thought about it for some time and decided to update it to pass in a prop onlyShowFolders which will be used to decide which query to used
const queryFn = onlyShowFolders
? trpc.resource.getFolderChildrenOf.useInfiniteQuery
: trpc.resource.getChildrenOf.useInfiniteQueryWhy this is an improvement:
- Centralized Updates: If there are changes to the
getFolderChildrenOforgetChildrenOfprops, we only need to update them in a single location (this file). Most of the changes will be confined to updating the props, which simplifies maintenance and enhances extensibility without over-engineering the solution. - Flexibility in Handling Query Changes: If the interfaces of the queries change significantly, we can simply create a copy and preserve the original query. This approach avoids forcing them to reuse the same query, providing more flexibility as the system evolves.
- Simplified Parent Component Props: The parent component no longer needs to be aware of which specific query to pass in. This reduces complexity and decouples the parent from needing to handle the specifics of the query logic.
apps/studio/src/components/ResourceSelector/ResourceSelector.tsx
Outdated
Show resolved
Hide resolved
| useEffect(() => { | ||
| if (curResourceId) { | ||
| onChange(curResourceId) | ||
| } | ||
| }, [curResourceId]) |
There was a problem hiding this comment.
clarification needed - i don't quite understand this section here, i'd expect to see teh onChange on a button as that's the trigger for the change rather than on useEffect, as that implies we're syncing state somewhere
There was a problem hiding this comment.
in some cases, the parent component that uses ResourceSelector will need onChange (props passed in) to trigger state change
…internal-links-for-folders
|
converting to draft first - need to update it to show full permalink of the resource instead of the permalink only |
…internal-links-for-folders
Problem
Closes https://linear.app/ogp/issue/ISOM-1659/internal-links-for-folders
Solution
Breaking Changes
Features:
getAncestrytogetAncestryWithSelfto fit current logic forresourceStackImprovements:
Screen.Recording.2024-11-07.at.6.29.36.PM.mov