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

Chore/2.0.0 rc.1 #448

Merged
merged 98 commits into from
May 30, 2024
Merged

Chore/2.0.0 rc.1 #448

merged 98 commits into from
May 30, 2024

Conversation

Baroshem
Copy link
Collaborator

@Baroshem Baroshem commented May 10, 2024

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

Closes #441
Closes #447
Closes #444
Closes #446
Closes #433
Closes #232

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes (if not applicable, please state why)

feat(core) : owasp default values
@vejja
Copy link
Collaborator

vejja commented May 23, 2024

Wonderful !
Looking forward to RC release 🎉🎉🎉

@vejja
Copy link
Collaborator

vejja commented May 29, 2024

Hi @GalacticHypernova
We have a typescript error on type OptionalThrowError<T> = Pick<T, 'throwError'>; which prevents the release.

src/types/module.ts(9,38): error TS2344: Type 'string' does not satisfy the constraint 'keyof T'.

Would you mind having a look at this ?

@GalacticHypernova
Copy link
Contributor

GalacticHypernova commented May 29, 2024

Would you mind having a look at this ?

Looks like TS needs a stricter type constraint. I believe doing something like this will solve it

type OptionalThrowError<T extends Record<string, any>> = 'throwError' extends keyof T ? Pick<T, 'throwError'> : never;

@vejja
Copy link
Collaborator

vejja commented May 30, 2024

After review, I am setting all fields optional and falling back to default config values in nested route rules.

This is because Nuxt internally forces our NuxtSecurityRouteRules fields to be optional. There is nothing we can do about this, as the Nuxt Module Builder internally overwrite our types with a DeepPartial<> utility.

This allows the user to use optional fields, while our code thinks they are mandatory - leading to potential type errors. I prefer to let our code know that they are in fact optional, and provide a fallback value.

@GalacticHypernova
Copy link
Contributor

Sorry for the late response, sounds good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment