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

Fix Plugin type issue #14668

Merged
merged 5 commits into from
Nov 28, 2024
Merged

Fix Plugin type issue #14668

merged 5 commits into from
Nov 28, 2024

Conversation

mrmckeb
Copy link

@mrmckeb mrmckeb commented Oct 15, 2024

This is a quick fix to an issue where the types for PluginsConfig don't match those used in plugins like @tailwindcss/container-queries.

This was caught by TypeScript with exactOptionalPropertyTypes enabled, where TypeScript checks if undefined can be supplied as a value for optional types.

I felt that it made more sense to fix this here, as it makes the core types more flexible, as opposed to each plugin needing to fix this when/if they hit it.

@philipp-spiess
Copy link
Member

philipp-spiess commented Oct 18, 2024

@mrmckeb Thanks for the PR! This change looks reasonable but I’m not sure I understand what current issue this is fixing. I was looking at the @tailwindcss/container-queries plugin you referenced but I don't see it using a config: undefined somewhere 🤔

I’m asking mostly because I see we have a bunch of other optional properties in the types for v3 and I wonder if they all need to be updated or if maybe it's fine the way it is right now.

@mrmckeb
Copy link
Author

mrmckeb commented Oct 20, 2024

Hi @philipp-spiess,

No problem, I can add more detail :)

The types for the plugin, as per the DTS file, are:

declare const _default: {
    handler: import("tailwindcss/types/config").PluginCreator;
    config?: Partial<import("tailwindcss/types/config").Config> | undefined;
};
export = _default;

So in this stricter mode of TypeScript, this is flagged as an issue.

I'm not 100% sure where the undefined comes from, but it seems to be from an older version of Tailwind (insiders, from 10+ months ago), but I feel this is a probably still a good fix given that other plugins may have the same types.

@mrmckeb
Copy link
Author

mrmckeb commented Oct 31, 2024

Hi @philipp-spiess, any more feedback on this one?

@philipp-spiess philipp-spiess requested a review from a team as a code owner November 13, 2024 11:24
@philipp-spiess
Copy link
Member

@mrmckeb Hey! sorry for not getting back earlier. Do you mind adding a changelog entry, something along those lines?

- Ensure the TypeScript types for `PluginsConfig` allow `undefined` values ([#14668](https://github.com/tailwindlabs/tailwindcss/pull/14668))

I'm having some issues pushing to your branch directly 🙈

@mrmckeb
Copy link
Author

mrmckeb commented Nov 24, 2024

Sorry @philipp-spiess - I was a little sick last week and this dropped off of my plate.

I've updated the branch and added that entry in.

You do have permission to write to this branch, so I'm not sure why you couldn't sorry!
image

@philipp-spiess
Copy link
Member

@mrmckeb Thanks a ton! Will merge this in but given our current focus on v4, it might take a while before we do a new v3 release unless something urgent needs fixing.

@philipp-spiess philipp-spiess merged commit 2702cfc into tailwindlabs:main Nov 28, 2024
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.

2 participants