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

Make permalink functions' opts arg optional #114

Conversation

mvargeson
Copy link
Contributor

@mvargeson mvargeson commented Apr 8, 2022

The new safariReaderFix opt for headerLink and the opts arg itself for all permalink functions are required in the types. It does not look like that matches reality in the implementation as defaults are applied to these. This changes the types definition to mark all of these as optional.

If this does not appear to be the case, then the headerLink section of the README should be updated to remove the headerLink() example shown without an opts arg.

I'm not sure what appropriate contribution guidelines are or if there are specific tests to run so largely looking for feedback on if this is a reasonable change and if there are any quality checks I need to perform.

@valeriangalliat valeriangalliat merged commit e10d169 into valeriangalliat:master Apr 8, 2022
@valeriangalliat
Copy link
Owner

Thanks a lot @mvargeson, that's a great catch, I forgot the ? when I added the safariReaderFix option to the types earlier today.

It's also 100% correct that the opts argument is optional for permalink functions, thanks for updating the types to also reflect that 🙏

I released your fix as part of v8.6.2

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.

None yet

2 participants