-
-
Notifications
You must be signed in to change notification settings - Fork 103
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
feat: Config.exclude can be array #559
Conversation
🦋 Changeset detectedLatest commit: ecf743a 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 |
@DanielHritcu is attempting to deploy a commit to the HoudiniGraphQL Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for submitting this @DanielHritcu! Mind updating the config docs page too? It lives in site/src/routes/api/config.svx.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
src/vite/index.ts
Outdated
@@ -42,7 +42,10 @@ export default function ({ | |||
} | |||
|
|||
// make sure that the file doesn't match the exclude | |||
return !config.exclude || !minimatch(filepath, config.exclude) | |||
return ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize this was there before, but I think we should try to use config.includeFile
here if possible
I added the code for include, but I can't manage to build the integration project to test. I get this weird error, and can't figure out why it's thrown. The files get included, and the artifacts generated. Can be replicated until my commit here : f149302 The error I get:
|
@DanielHritcu are you still running into that error? |
@AlecAivazis yes I am. |
Oh, interesting it's happening on github too - alright so it must be related to your changes since these tests ran successfully in your other PR. I'll try to figure out what's going on |
@DanielHritcu I pushed up a few changes that I think have fixed the problem. I'm really not sure how it's any different from what you have but i'm not getting the errors any more. Let me know what you think |
* feat: Config.exclude can be array * make include array * throw if exclude all * remove debug console.log * use config.includeFile in vite plugin * exclude as array * reset * invert exclude length check * invert exclude check * clean up exclude default value * changeset * fix bug hiding source files * add docs * unique source files Co-authored-by: Alec Aivazis <[email protected]>
In progress
Fixes #558
Allow passing an array of strings to
Config.exclude
. Also allow string for this to be a non breaking change.Should
Config.include
support arrays as well ?To help everyone out, please make sure your PR does the following:
pnpm run tests
andcd integration && pnpm run tests
pnpm changeset