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

Support ?query in ids #161

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

edemaine
Copy link

@edemaine edemaine commented Nov 11, 2024

Several Vite plugins, including Civet's, generate artificial ids with a ?query at the end. This should be ignored when detecting extensions, and indeed getExtension already did this. (I tweaked it slightly to also support ? followed by no query, though that probably doesn't happen in practice.) But the .[mc]?[tj]sx extension detection failed to actually use this extension. Now it does.

As Ryan correctly guessed in Discord, this was first introduced in af0a436 — it affects every version after 2.8.3.

Copy link

changeset-bot bot commented Nov 11, 2024

⚠️ No Changeset found

Latest commit: 7b9e837

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@yume-chan
Copy link

In fact I tried to fix this in #154, for the https://github.com/solidjs/vite-plugin-solid/tree/next branch, but it's not merged back to main

@edemaine
Copy link
Author

Oh, indeed. @ryansolid it looks like 2.11.0 still doesn't have either form of this change (presumably it was released from main?), so still can't be used with Civet.

I notice that there's a potential bug in both versions, which is that getExtension removes the ? query after finding the last ., so if there's a . after the ?, it won't do the right thing.

Also, #154 (currently on next) is a bit inefficient in that it does the ? removal twice (once in getExtension and once in containsSolidField.

Let me know if I should change this PR to be relative to next instead... I didn't realize there were two competing branches.

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