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

feat: allow argument directives to be called even if the argument is null #3233

Merged

Conversation

clayne11
Copy link
Contributor

The existing implementation assumes that if an input argument is null, you don't want to call the directive. This is a very constraining assumption — directives may want to not just mutate an argument but to actually outright set it.

This is a breaking change as argument directives now need to handle null input values. Added a new config switch:

call_argument_directives_with_nulls: bool

to control this new behavior.

Describe your PR and link to any relevant issues.

I have:

  • Added tests covering the bug / feature (see testing)
  • Updated any relevant documentation (see docs)

This is unreverting #3188 after fixing the issues with that PR.

…null (99designs#3188)

The existing implementation assumes that if an input argument is null, you don't want to call the directive. This is a very constraining assumption — directives may want to not just mutate an argument but to actually outright set it.

This is a breaking change as argument directives now need to handle null input values. Added a new config switch:

call_argument_directives_with_nulls: bool

to control this new behavior.
@coveralls
Copy link

coveralls commented Aug 27, 2024

Coverage Status

coverage: 56.634% (-17.8%) from 74.454%
when pulling 1a928ef on uber:curtis.layne/always-call-directives
into 3e76e7e on 99designs:master.

@StevenACoffman StevenACoffman merged commit 18378f9 into 99designs:master Aug 28, 2024
16 of 17 checks passed
@StevenACoffman
Copy link
Collaborator

Thanks! A config option so that people can opt-in to this behavior is much appreciated.

@clayne11 clayne11 deleted the curtis.layne/always-call-directives branch August 28, 2024 14:18
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