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

Add Schema old behaviour back via Options #960

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

Conversation

iamandrewluca
Copy link

@iamandrewluca iamandrewluca commented Sep 27, 2024

Allow Schema behavior before this PR #887 via Options. Please see PR comments for more context.

@sindresorhus
Copy link
Owner

I think it makes more sense to add an option to the Scheme type. The behavior introduced here does not really make it "loose" and "loose" is also a ambiguous term for this.

@grigorischristainas
Copy link
Contributor

@iamandrewluca @sindresorhus if you agree, I would suggest adding the optional config { recurseIntoArrays: boolean } (defaulting to true). This way, the type would eventually be used as follows:

type FooOption = 'A' | 'B';
type FooSchema = Schema<typeof foo, FooOption, {recurseIntoArrays: false}>;

@iamandrewluca
Copy link
Author

I will try to refactor it, to be able to use recurseIntoArrays

@grigorischristainas
Copy link
Contributor

Ok, let me know if you need any assistance, I'd be glad to help.

@iamandrewluca
Copy link
Author

iamandrewluca commented Oct 11, 2024

@grigorischristainas I tried the approach, but I think something I didn't do right.

I think I figured it out. Checked other types.

I modified the current tests just to check. In the end, I will write separate type tests for this use case.
schema-loose will delete when we figure schema out.

@iamandrewluca
Copy link
Author

I think I figured it out. Checked other types.

Figured it out with the options. Can't figure out how to make it work ))
I feel I'm missing some TypeScript gotcha.

@grigorischristainas
Copy link
Contributor

Thanks @iamandrewluca for the update, I will be able to take a look by the end of day tomorrow, or at the latest on Monday. I could checkout at your branch and commit any changes there.

@iamandrewluca iamandrewluca changed the title Add Schema old behaviour back as SchemaLoose Add Schema old behaviour back via Options Oct 12, 2024
@grigorischristainas
Copy link
Contributor

@iamandrewluca I cloned your branch (revert-schema-as-schema-loose), but I can't push my commits, could you please give me that permission? I think you can enable that from Settings -> Collaborators.

@iamandrewluca
Copy link
Author

iamandrewluca commented Oct 14, 2024

@grigorischristainas invited.

If you want you can fork my fork, and open the PR in my fork targeting current branch

@grigorischristainas
Copy link
Contributor

Unfortunately, I cannot fork it because I already have a fork of the original type-fest repository. Maybe I could create a new branch in your forked repo and if you agree with the changes then merge it into yours.

@iamandrewluca
Copy link
Author

I invited you as a collaborator in my fork.

@grigorischristainas
Copy link
Contributor

Thanks, I opened a PR to your forked repo with the changes. Please let me know if you need any additional information.

@iamandrewluca
Copy link
Author

@grigorischristainas, thanks for the help, I merged your PR. All the changes are visible here now.

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.

3 participants