-
Notifications
You must be signed in to change notification settings - Fork 2
ComponentSpecProvider Computes Digest #1438
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think digest is technically part of the component spec. I wonder if we just update the component spec component to generate a component spec for itself instead of having it in context, that way we keep the context a little cleaner and it removes the effect in favour of tanstackQuery.
Here is a vibe coded example. Also this isn't a hill im going to die on, just a suggestion
// src/components/Editor/ComponentDigest.tsx
import { useSuspenseQuery } from "@tanstack/react-query";
import { Suspense, useMemo } from "react";
import { Button } from "@/components/ui/button";
import { Skeleton } from "@/components/ui/skeleton";
import useToastNotification from "@/hooks/useToastNotification";
import { useComponentSpec } from "@/providers/ComponentSpecProvider";
import { generateDigest } from "@/services/componentService";
import { componentSpecToYaml } from "@/utils/componentStore";
function useComponentSpecDigest() {
const { componentSpec } = useComponentSpec();
const yamlText = useMemo(
() => componentSpecToYaml(componentSpec),
[componentSpec],
);
const { data } = useSuspenseQuery({
queryKey: ["componentSpec", "digest", yamlText],
queryFn: () => generateDigest(yamlText),
staleTime: Infinity,
});
return data;
}
const DigestSkeleton = () => (
<div class="">
<h3 class="">Digest</h3>
</div>
);
const DigestContent = () => {
const digest = useComponentSpecDigest();
const notify = useToastNotification();
return (
<div class=""><h3 class="">Digest</h3><ButtonclassName="bg-gray-100 border border-gray-300 rounded p-2 h-fit text-xs w-full text-left hover:bg-gray-200 active:bg-gray-300 transition cursor-pointer"onClick={() => {navigator.clipboard.writeText(digest);notify("Digest copied to clipboard", "success");}}variant="ghost"><span class="">{digest}</span></div>
);
};
export const ComponentDigest = () => (
\<Suspense fallback={\<DigestSkeleton />}>
\<DigestContent />
\</Suspense>
); There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did consider whether to do this calculation in the component itself vs the provider. I ultimately put it in the provider because any given component spec is going to have its own unique digest so I thought it might be useful to have one source of truth for the digest.
I'm not sure about using useQuery because the digest is constantly changing so the benefits of caching is low. I think for now I'm going to keep it as-is and we can take a closer look later.
afafd58 to
a04fb3d
Compare
71b05e1 to
c3005a6
Compare
Mbeaulne
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still don't think its needed in the componentSpecProvider, but thats not going to stop be from approving! But if Max feels any way, lets discuss then.
c3005a6 to
cac5315
Compare
|
I will merge and we can follow up later if we decide to go in a different direction. |

Description
Fixes a bug where pipeline digest did not update in realtime when changes were made to a pipeline.
ComponentSpecProvider now exports a
digestcalculated dynamically from the spec.Related Issue and Pull requests
Closes https://github.com/Shopify/oasis-frontend/issues/368
Type of Change
Checklist
Screenshots (if applicable)
Test Instructions
Verify that pipeline digest updates when changes are made to a pipeline.
Verify that digest is the same after refreshing or reloading a pipeline.
Additional Comments