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

fix(core): handle undefined properties in schemas with additionalProperties #22426

Merged
merged 2 commits into from
Mar 27, 2024

Conversation

MaxKless
Copy link
Collaborator

@MaxKless MaxKless commented Mar 21, 2024

fixes #22349

@MaxKless MaxKless requested a review from a team as a code owner March 21, 2024 07:42
@MaxKless MaxKless requested a review from AgentEnder March 21, 2024 07:42
Copy link

vercel bot commented Mar 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated (UTC)
nx-dev ⬜️ Ignored (Inspect) Visit Preview Mar 27, 2024 10:04am

@robertIsaac
Copy link
Contributor

thanks for the fix :)

@noelsoong
Copy link

i don't like the idea of using the nullish operator and giving it a default empty object.

i think the schema.additionalProperties !== undefined could be changed to

schema.additionalProperties != null

@noelsoong
Copy link

nvm addditionalProperties is not .properties carry on XD

@mattfysh
Copy link

This does prevent the error from appearing, but I think there is a larger issue at play here because after applying this fix the property that was triggering this error is still ignored. To reproduce:

npx create-nx-workspace@latest xyz \
  --workspaceType integrated --ci skip --preset ts --packageManager pnpm
cd xyz
nx g lib foo --bundler=none --unit-test-runner=none

foo/project.json

"printenv": {
      "executor": "nx:run-commands",
      "options": {
        "command": "env",
        "env": {
          "ONE": "TWO"
        }
      }
    },

Without this fix applied, you'll see the Cannot convert undefined or null to object

After applying the fix, the error no longer appears, but the property that triggered the error (env) is not applied to the task. You can see the ONE env var is missing in the logged output

@MaxKless MaxKless force-pushed the schema-additionalproperties-bug branch from b0d8cc8 to 29d1012 Compare March 27, 2024 09:54
@MaxKless
Copy link
Collaborator Author

thanks @mattfysh for the catch & the repro. Super helpful!
It's an unrelated issue but I fixed it and added it to the PR :)

@MaxKless MaxKless force-pushed the schema-additionalproperties-bug branch from 29d1012 to e921535 Compare March 27, 2024 10:02
@FrozenPandaz FrozenPandaz merged commit bb4dd7d into master Mar 27, 2024
6 checks passed
@FrozenPandaz FrozenPandaz deleted the schema-additionalproperties-bug branch March 27, 2024 16:27
@robertIsaac
Copy link
Contributor

hi @FrozenPandaz
do you plan to publish it as a bugfix for nx 18.1?

Copy link

github-actions bot commented Apr 4, 2024

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid JSON Schema enforcement of additionalProperties
5 participants