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

The returned properties of MultilinkStoryblok do not match generated type, and should be nullable. #115

Closed
Joobs opened this issue Sep 12, 2024 · 2 comments · Fixed by #122
Labels
bug Something isn't working help wanted [Contribution] Extra attention is needed p2-nice-to-have [Priority] Lower priority, beneficial enhancements that are not urgent. PR welcome [Contribution] Pull requests are welcome.

Comments

@Joobs
Copy link

Joobs commented Sep 12, 2024

Similar to these issues 112 and 113, The type of MultilinkStoryblok (generated by generate-sb-types) does not match the JSON data returned by the API.

export type MultilinkStoryblok =
  | {
      id?: string;
      cached_url?: string;
      anchor?: string;
      linktype?: "story";
      target?: "_self" | "_blank";
      story?: {
        name: string;
        created_at?: string;
        published_at?: string;
        id: number;
        uuid: string;
        content?: {};
        slug: string;
        full_slug: string;
        sort_by_date?: null | string;
        position?: number;
        tag_list?: string[];
        is_startpage?: boolean;
        parent_id?: null | number;
        meta_data?: null | {};
        group_id?: string;
        first_published_at?: string;
        release_id?: null | number;
        lang?: string;
        path?: null | string;
        alternates?: unknown[];
        default_full_slug?: null | string;
        translated_slugs?: null | unknown[];
      };
    }
  | {
      url?: string;
      cached_url?: string;
      anchor?: string;
      linktype?: "asset" | "url";
      target?: "_self" | "_blank";
    }
  | {
      email?: string;
      linktype?: "email";
      target?: "_self" | "_blank";
    };

The returned JSON for a link contain additional properties. I've commented out the properties in question

// STORY LINK:

{
  id: "123456",
  // url: "",
  linktype: "story",
  // fieldtype: "multilink",
  cached_url: "/test",
  // prep: true,
  story: {
    name: "Example Title",
    id: 123456,
    uuid: "123456",
    slug: "test",
    // url: "test",
    full_slug: "test",
    // _stopResolving: true
  },
};


// EMPTY STORY LINK

{
	id: "",
	// url: "",
	linktype: "story",
	// fieldtype: "multilink",
	cached_url: "",
	// prep: true
};

// URL LINK:

{
  // id: "",
  url: "/a-test-url",
  linktype: "url",
  // fieldtype: "multilink",
  cached_url: "/a-test-url",
};


// EMPTY URL LINK:

{
  // id: "",
  url: "",
  linktype: "url",
  // fieldtype: "multilink",
  cached_url: "",
};

And similar to the AssetStoryBlok issues (mentioned earlier), why does does an blank link field return an object, and not null/undefined? To identify a blank field I have to check the value of the linkfield and then based on that check if either the url or id is blank (depending on if it's a link to a URL or a Story).

Unrelated to this ticket, the Rich Text fields also return an empty object, and requires additional checks to discover if it's blank.

@Joobs Joobs added the bug Something isn't working label Sep 12, 2024
@alvarosabu alvarosabu added help wanted [Contribution] Extra attention is needed p2-nice-to-have [Priority] Lower priority, beneficial enhancements that are not urgent. PR welcome [Contribution] Pull requests are welcome. labels Sep 17, 2024
@Edo-San Edo-San linked a pull request Oct 2, 2024 that will close this issue
5 tasks
@Edo-San
Copy link
Contributor

Edo-San commented Oct 2, 2024

Hi @Joobs thank you for reporting these inconsistencies and for taking the time to file a detailed comparison. Much appreciated!
I have filed a PR #122 that should fix this 🙏

As for the question related to the Asset field type - which is returning a full object instead of a null - that's because of backward compatibility reasons. But you have a point, indeed.
What if we provided a 1st-party way to check those empty values? Something like an isAssetEmpty(asset) function - which is nothing short of what you probably already did - exported by our JS libraries? Do you think you would use it and/or do you think it would improve your developer experience?

@LukasHechenbergerID
Copy link

As for the question related to the Asset field type - which is returning a full object instead of a null - that's because of backward compatibility reasons. But you have a point, indeed. What if we provided a 1st-party way to check those empty values? Something like an isAssetEmpty(asset) function - which is nothing short of what you probably already did - exported by our JS libraries? Do you think you would use it and/or do you think it would improve your developer experience?

Hello, I would much appreciate something like that!! Because the type checking is a bit annyoing with the new PR. Could you post how you would implement that in a clean way? I would just copy it for the time being, but it would be good to have in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted [Contribution] Extra attention is needed p2-nice-to-have [Priority] Lower priority, beneficial enhancements that are not urgent. PR welcome [Contribution] Pull requests are welcome.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants