Skip to content

Update json5 dependency #6937

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

Merged
merged 4 commits into from
Jun 6, 2024
Merged

Conversation

Swiddis
Copy link
Contributor

@Swiddis Swiddis commented Jun 6, 2024

Description

Coming from downstream where we recently added json5 for a feature, our distribution build is failing due to an outdated json5 version used here. We could downgrade our dependency, but considering that the json5 changelog has no breaking changes for the core API, it seems more straightforward to update it here directly.

Issues Resolved

Resolves build failures in opensearch-project/dashboards-observability#1861.

Screenshot

N/A

Testing the changes

Trusting CI to catch any issues on this one; according to the json5 changelog this is a no-op.

Changelog

  • skip: update json5 dependency from 1.0.1 to 2.2.3

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Signed-off-by: Simeon Widdis <[email protected]>
Copy link
Contributor

github-actions bot commented Jun 6, 2024

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "chore". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

Signed-off-by: Simeon Widdis <[email protected]>
Copy link
Contributor

github-actions bot commented Jun 6, 2024

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "chore". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

@Swiddis
Copy link
Contributor Author

Swiddis commented Jun 6, 2024

Invalid description prefix. Found "chore". Only "skip" entry option is permitted for manual commit of changeset files.

The PR template says chore is permitted -- what should I use instead?

Nevermind, I re-read the second sentence. Okay, can't get it to work with chore (even without a manual changelog commit), I'll just use skip since this is sufficiently small.

Copy link
Contributor

github-actions bot commented Jun 6, 2024

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "chore". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

@github-actions github-actions bot added Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry and removed failed changeset labels Jun 6, 2024
This reverts commit 526e0bb.

Signed-off-by: Simeon Widdis <[email protected]>
Copy link
Contributor

github-actions bot commented Jun 6, 2024

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "chore". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

@github-actions github-actions bot added failed changeset Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry and removed Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry failed changeset labels Jun 6, 2024
Copy link

codecov bot commented Jun 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.42%. Comparing base (48144c8) to head (499f070).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6937      +/-   ##
==========================================
- Coverage   67.43%   67.42%   -0.02%     
==========================================
  Files        3443     3443              
  Lines       67806    67806              
  Branches    11031    11031              
==========================================
- Hits        45727    45717      -10     
- Misses      19413    19423      +10     
  Partials     2666     2666              
Flag Coverage Δ
Linux_1 33.09% <ø> (ø)
Linux_2 55.06% <ø> (ø)
Linux_3 45.20% <ø> (ø)
Linux_4 34.88% <ø> (ø)
Windows_1 ?
Windows_2 55.03% <ø> (ø)
Windows_3 45.21% <ø> (ø)
Windows_4 34.88% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ZilongX ZilongX merged commit 9674147 into opensearch-project:main Jun 6, 2024
67 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jun 6, 2024
* Update json5

Signed-off-by: Simeon Widdis <[email protected]>

* Update changelog

Signed-off-by: Simeon Widdis <[email protected]>

* Revert "Update changelog"

This reverts commit 526e0bb.

Signed-off-by: Simeon Widdis <[email protected]>

---------

Signed-off-by: Simeon Widdis <[email protected]>
Co-authored-by: Lu Yu <[email protected]>
(cherry picked from commit 9674147)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@ananzh
Copy link
Member

ananzh commented Jun 6, 2024

This one is tricky

ubuntu@ip-172-31-1-206:~/work/OpenSearch-Dashboards-2$ yarn why json5
yarn why v1.22.19
[1/4] Why do we have the module "json5"...?
[2/4] Initialising dependency graph...
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "typescript@~4.5.2"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Lockfile has incorrect entry for "[email protected]". Ignoring it.
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
warning Resolution field "[email protected]" is incompatible with requested version "[email protected]"
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "[email protected]"
info Has been hoisted to "json5"
info Reasons this module exists
   - "workspace-aggregator-c7a7f120-029f-4694-b63a-a2dbb4f3f0f1" depends on it
   - Specified in "devDependencies"
   - Hoisted from "_project_#@babel#core#json5"
   - Hoisted from "_project_#json5"
   - Hoisted from "_project_#istanbul-lib-instrument#@babel#core#json5"
   - Hoisted from "_project_#@osd#ui-shared-deps#loader-utils#json5"
   - Hoisted from "_project_#babel-jest#@jest#transform#@babel#core#json5"
   - Hoisted from "_project_#nyc#istanbul-lib-instrument#@babel#core#json5"
   - Hoisted from "_project_#jest#@jest#core#jest-snapshot#@babel#core#json5"
   - Hoisted from "_project_#jest#@jest#core#jest-config#@babel#core#json5"
info Disk size without dependencies: "292KB"
info Disk size with unique dependencies: "292KB"
info Disk size with transitive dependencies: "292KB"
info Number of shared dependencies: 0
=> Found "tsconfig-paths#[email protected]"
info This module exists because "_project_#eslint-plugin-import#tsconfig-paths" depends on it.
info Disk size without dependencies: "112KB"
info Disk size with unique dependencies: "208KB"
info Disk size with transitive dependencies: "208KB"
info Number of shared dependencies: 1
Done in 1.41s.

tsconfig-paths 3.4.1 uses [email protected] and it can't bump json5 due to breaking change jonaskello/tsconfig-paths#173

Therefore we should never add resolution to json5 in the future.

The change only impacts our dep and I compared. we only use stringfy from JSON5 in json5. Here is v1's stringfy https://github.com/json5/json5/blob/v1/src/stringify.js and here is v2's https://github.com/json5/json5/blob/v2/lib/stringify.js.
Functionality wise no difference. But v1 is CommonJs, v2 is not any more.

Therefore, the change is good to me.

BionIT added a commit that referenced this pull request Jun 6, 2024
* Update json5



* Update changelog



* Revert "Update changelog"

This reverts commit 526e0bb.



---------



(cherry picked from commit 9674147)

Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Lu Yu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x first-time-contributor Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry v2.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants