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

Updated js-yaml to v4 #190678

Merged
merged 18 commits into from
Sep 19, 2024
Merged

Conversation

elena-shostak
Copy link
Contributor

@elena-shostak elena-shostak commented Aug 19, 2024

Summary

Updated js-yaml to 4.1.0.

This PR also introduces a type override for the js-yaml load function to maintain compatibility across the codebase. Specifically, updated type definition of the load function looks as follows:

function load<T = any>(str: string, opts?: jsyaml.LoadOptions): T;

The original type definition of the load function in js-yaml changed from any to unknown. This change would require extensive type updates throughout the entire repository to accommodate the unknown type. To avoid widespread type changes and potential issues in the codebase, the type is overriden back to any for now.
This is a temporary measure, we plan to address the necessary type changes in subsequent PRs, where teams will gradually update the codebase to work with the unknown type.

Checklist

Release note

Updated js-yaml to 4.1.0.

@elena-shostak
Copy link
Contributor Author

/ci

4 similar comments
@elena-shostak
Copy link
Contributor Author

/ci

@elena-shostak
Copy link
Contributor Author

/ci

@elena-shostak
Copy link
Contributor Author

/ci

@elena-shostak
Copy link
Contributor Author

/ci

@elena-shostak elena-shostak force-pushed the 190624-js-yaml-upgrade branch from cc25199 to c6c985a Compare August 20, 2024 10:43
@elena-shostak
Copy link
Contributor Author

/ci

1 similar comment
@elena-shostak
Copy link
Contributor Author

/ci

@elena-shostak
Copy link
Contributor Author

@elasticmachine merge upstream

@elena-shostak
Copy link
Contributor Author

/ci

1 similar comment
@elena-shostak
Copy link
Contributor Author

/ci

yarn.lock Outdated
@@ -21676,7 +21676,7 @@ [email protected], js-yaml@^4.1.0:
dependencies:
argparse "^2.0.1"

js-yaml@^3.13.1, js-yaml@^3.14.1:
js-yaml@^3.13.1:
Copy link
Contributor Author

@elena-shostak elena-shostak Aug 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is coming from @istanbuljs/load-nyc-config, we can also set a resolution, though seems negligible for now.

"**/@istanbuljs/load-nyc-config/**/js-yaml": "4.1.0"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. As this is a transitive development dependency, I think it is ok to leave this for now.

@elena-shostak elena-shostak added Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! release_note:enhancement labels Aug 21, 2024
@elena-shostak elena-shostak marked this pull request as ready for review August 21, 2024 15:02
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

@elena-shostak elena-shostak requested review from a team as code owners August 21, 2024 15:02
@elena-shostak elena-shostak requested a review from a team August 21, 2024 15:02
@kibana-ci
Copy link
Collaborator

kibana-ci commented Sep 18, 2024

💛 Build succeeded, but was flaky

  • Buildkite Build
  • Commit: 46dc19c
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-190678-46dc19cbb59a

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
apm 1871 1865 -6
cloudDefend 110 104 -6
fleet 1213 1207 -6
total -18

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
apm 3.5MB 3.4MB -130.7KB
cloudDefend 231.9KB 101.3KB -130.6KB
fleet 1.8MB 1.7MB -130.5KB
total -391.9KB

History

  • 💔 Build #234978 failed 3cab99a5cc0984f2d2715afc80c6a85e7825fa6b
  • 💔 Build #234963 failed 375d519b4c4793527fa114b98cca6e42bee26e07
  • 💔 Build #233623 failed 37ad472b90f9a8bf7967d0ed441fc94689de4503
  • 💔 Build #233379 failed 1cbc5a3168c76023dd9651d04b5ee1eb3e76a839
  • 💔 Build #233352 failed 3c0aa9a49617e72723b807cd74d1688c3dd05140

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link

@vgomez-el vgomez-el left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@elena-shostak elena-shostak merged commit 28aa274 into elastic:main Sep 19, 2024
49 checks passed
@kibanamachine kibanamachine added v9.0.0 backport:skip This commit does not require backporting labels Sep 19, 2024
delanni added a commit that referenced this pull request Sep 19, 2024
## Summary
Main seems to be broken because of a check. These are probably
regenerated with a different shape since the js-yaml update: #190678
delanni added a commit that referenced this pull request Sep 19, 2024
## Summary
More files to be regenerated with a different shape since the js-yaml
update: #190678
elena-shostak added a commit that referenced this pull request Sep 20, 2024
## Summary

Since js-yaml update has been merged in
#190678 we don't need
`no_unsafe_js_yaml ` anymore
@elena-shostak elena-shostak deleted the 190624-js-yaml-upgrade branch October 9, 2024 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:review backport:skip This commit does not require backporting ci:project-deploy-observability Create an Observability project release_note:enhancement Team:Fleet Team label for Observability Data Collection Fleet team Team:Obs AI Assistant Observability AI Assistant Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team Team:obs-ux-management Observability Management User Experience Team Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.