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

Expose 'type-fest' as dependency instead of devDependency #120

Closed
BillyBlaze opened this issue Feb 4, 2022 · 2 comments · Fixed by #121
Closed

Expose 'type-fest' as dependency instead of devDependency #120

BillyBlaze opened this issue Feb 4, 2022 · 2 comments · Fixed by #121
Assignees
Labels
bug Something isn't working

Comments

@BillyBlaze
Copy link

Currently this package makes use of the type-fest package, for example this piece of code uses the PascalCase type from that package. However the dependency of type-fest is set underneath devDependencies while technically this should be underneath dependencies or in the worst case as a peerDependency.

The current situation is problematic for our build (we're using Bazel) we're getting build errors such as:

BUILD.bazel:6:11: in ts_project rule //packages/libs/prismic:tsconfig: target '@npm//type-fest:type-fest' is not visible from target '//packages/libs/prismic:tsconfig'. Check the visibility declaration of the former target if you think the dependency is legitimate

The workaround to get this working is to add type-fest to our package.json as a dependency, however this is a bad practice. (e.g. mismatch in versions)

Therefor my question; Can we move this to dependencies?

@BillyBlaze BillyBlaze added the bug Something isn't working label Feb 4, 2022
@angeloashmore angeloashmore self-assigned this Feb 4, 2022
@angeloashmore
Copy link
Member

angeloashmore commented Feb 5, 2022

Hey @BillyBlaze, thanks for pointing this out. I've moved the type used from type-fest (PascalCase) into the package so we don't need it as a dependency. You can see the change in this PR: #121

This fix will be published in the next release which should come out early next week. I'll post here again once it's published. Thanks!

Edit: This was published today as part of v2.0.6.

@BillyBlaze could you update and let me know if this fixes the issue on your end? Thanks!

@BillyBlaze
Copy link
Author

Hi @angeloashmore, your welcome and thanks for your quick response! Removing the dependency is even a better solution 😄 I have updated my project and everything is good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants