update ChangeFreq to support typescript configurations with string literal

[vic1707]

ATM if you wish to configure the integration in a typescript file like so you get an error :

/* ... */
sitemap({
  changefreq: 'daily' // Type '"daily"' is not assignable to type 'EnumChangefreq | undefined'. ts(2322)
})
/* ... */

This is due to how enums (and types made from enums) work in typescript.

To get it working the user has to use an as ChangeFreq which isn't great.

changefreq: 'daily' as ChangeFreq

Or he can access EnumChangefreq by adding sitemap as a project dependency

changefreq: EnumChangefreq.DAILY

I'm not a big fan of it since sitemap is a dependency of this integration (could lead to version mismatch).

---

I'd like to get autocomplete without risking an version mismatch, to do so I thought of two solutions :

• re-exporting the EnumChangefreq enum from sitemap.
• the proposed change.

Re-exporting the enum will force the config to be changefreq: EnumChangefreq.<DURATION> and prevent the use of a string literal (and the ChangeFreq becomes useless). It's fine but would, IMO, require a note in the readme and an edit to the CLI astro add sitemap.

The proposed change allows to set changefreq with a string literal or via ChangeFreq.<DURATION>.

Testing

I simply tested it by modifying the index.d.ts inside the node_modules of one of my project. Since the type is never used outside of the config type definition it can't really affect anything.
It wasn't enough hence the force push and failed first build, sorry about that.

Docs

I don't think this change need any documentation since everything should be transparent to the user since it's just adding support for the string literal.

[changeset-bot]

🦋 Changeset detected

Latest commit: d953771

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

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

[vic1707]

this can be technically be replaced with

Suggested change

	changefreq?: EnumChangefreq;
	changefreq?: typeof ChangeFreq[keyof typeof ChangeFreq];

if we don't want to reuse EnumChangefreq but EnumChangefreq is way cleaner and readable

[natemoo-re]

This seems reasonable to me! cc @Princesseuh for a TypeScript sanity check.

Seems like allowing Lowercase<keyof typeof EnumChangefreq> would be another way to go here, but might not always be correct if the sitemap package changes the internal logic.

[vic1707]

I have to say I didn't think about the Lowercase<keyof typeof EnumChangefreq> trick, I'm liking it more than what I did!
Wondering from a Typescript standpoint which version is better 🤔

[natemoo-re]

I'm cool with either solution, but the { ...enum } hack does feel a bit weird since that requires a runtime change purely for better types.

I wonder if we can use a utility type like this?

type EnumOrValue<E extends Record<string|number|symbol, any>> = E | `${E[keyof E]}`;

type ChangeFreq = EnumOrValue<typeof EnumChangefreq>

[vic1707]

I frankly prefer avoiding the { ...enum } now that you showed me a better type solution.

The utility type also work (if using unknown instead of any doesn't break anything) but creating a utility type for that one thing might make things more complicated for someone reading the typedefinitions, no ?

[Princesseuh]

This seems reasonable to me! cc @Princesseuh for a TypeScript sanity check.

Seems like allowing Lowercase<keyof typeof EnumChangefreq> would be another way to go here, but might not always be correct if the sitemap package changes the internal logic.

I'm not sure if I 100% get it, but you can just use

`${EnumChangefreq}`

as a type, and it'll accept both using the enum directly and the value. And you get auto completion as well

(my real sanity check here is that enums in TypeScript are Not-So-Good:tm:)

[natemoo-re]

I'm not sure if I 100% get it, but you can just use

`${EnumChangefreq}`

as a type, and it'll accept both using the enum directly and the value. And you get auto completion as well

(my real sanity check here is that enums in TypeScript are Not-So-Good™️)

D'oh. That is way easier than either of our suggestions—thanks!

[vic1707]

Completely agree about enums not being good.
Thanks for the suggestion, it should work like a charm !

[vic1707]

build error came from
src/generate-sitemap.ts:27:8 which lists the SitemapItem (integration type) which depends on sitemap SitemapItemLoose & SitemapItemBase which declare changefreq?: EnumChangefreq.

[vic1707]

Since I finally got back access to my pc I tested @natemoo-re suggestions

type ChangeFreq = Lowercase<keyof typeof EnumChangefreq>

and

type EnumOrValue<E extends Record<string|number|symbol, any>> = E | `${E[keyof E]}`;

type ChangeFreq = EnumOrValue<typeof EnumChangefreq>

Unfortunately both caused the exact same error.

Did some more testing and even my solution doesn't actually work.
It passes build but doesn't fix the ts(2322).
Personal note: never trust modifying index.d.ts as a valid way of testing.

Marking the PR as draft for now

[vic1707]

only solution I could find is to use Princesseuh's solution and using a as in generate-sitemap.ts

      changefreq: changefreq as EnumChangefreq,

I don't like the fact of using an as but we're fighting a Typescript limitation/design choice + the fact that we don't have access to the enum declaration to change it.
Maybe a PR to sitemap would be cleaner 🤔
tell me what you think.

[vic1707]

only solution I could find is to use Princesseuh's solution and using a as in generate-sitemap.ts

      changefreq: changefreq as EnumChangefreq,

I don't like the fact of using an as but we're fighting a Typescript limitation/design choice + the fact that we don't have access to the enum declaration to change it.
Maybe a PR to sitemap would be cleaner 🤔
tell me what you think.

[natemoo-re]

I'm okay with this cast since it allows a better UX with the string literals! Would love one more approval before merging, just as a gut check.