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

Update dependency jsonpath-plus to v5 #3398

Merged
merged 4 commits into from
Feb 8, 2021

Conversation

renovate[bot]
Copy link
Contributor

@renovate renovate bot commented Jan 14, 2021

WhiteSource Renovate

This PR contains the following updates:

Package Change Age Adoption Passing Confidence
jsonpath-plus 4.0.0 -> 5.0.2 age adoption passing confidence

Release Notes

s3u/JSONPath

v5.0.2

Compare Source

  • Fix: Proper Node CommonJS export; fixes #​144

v5.0.1

Compare Source

  • Fix: Proper Node CommonJS export; fixes #​143
  • Docs: Properly indicate new browser paths

v5.0.0

Compare Source

  • Breaking change: Add type: 'commonjs' and exports: {import, require}
    (with node-import-test npm script to demo)
  • Breaking change: Change paths for browser (now is
    dist/index-browser-umd.js or dist/index-browser-es.js)
    (for Node, main and module point to new Node-specific dist)
  • Breaking enhancement: Add browser for browser bundling;
    allowing static analysis environments, doesn't have however
    conditional code to require vm); for ESM browser bundling,
    now must check browser in Rollup Node resolver plugin;
    see README
  • Build: Update per latest devDeps.
  • Docs: Add Regex (.match) example on value (@​jeffreypriebe)
  • Docs: Add Regex (.match) example on property
  • Docs: Fix XPath example (@​humbertoc-silva)
  • Docs: Link to XPath 2.0 tester
  • Docs: Update badges per latest updates
  • Linting: quote props
  • Linting: As per latest ash-nazg
  • Testing: Fix browser tests
  • Testing: Add test case for setting values in callbacks (issue #​126)
  • Testing: Add more at-sign tests
  • Testing: Bump timeout
  • Travis: Check Node 14
  • Travis: add default dist field to avoid extra config reporting
  • npm: Update from deprecated rollup-plugin-babel to @rollup/plugin-babel
    (and make babelHelpers explicit)
  • npm: Reorder scripts by test execution order
  • npm: Update devDeps

Renovate configuration

📅 Schedule: At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

♻️ Rebasing: Renovate will not automatically rebase this PR, because other commits have been found.

🔕 Ignore: Close this PR and you won't be reminded about this update again.


  • If you want to rebase/retry this PR, check this box

This PR has been generated by WhiteSource Renovate. View repository job log here.

@renovate renovate bot force-pushed the renovate/jsonpath-plus-5.x branch 3 times, most recently from a9a6ac1 to 2a34e0a Compare January 19, 2021 07:51
Copy link
Contributor

@cemalettin-work cemalettin-work left a comment

Choose a reason for hiding this comment

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

It seems like the JSONPATH function takes very long after the update, parsing is very slow which fails the tests. Can be a bug in the new version

@renovate renovate bot force-pushed the renovate/jsonpath-plus-5.x branch 2 times, most recently from c6ea081 to 7bcb5fd Compare January 28, 2021 09:12
@gjvoosten
Copy link
Collaborator

It seems like the JSONPATH function takes very long after the update, parsing is very slow which fails the tests. Can be a bug in the new version

Actually, it appears the semantics of some of the path lookups have changed with v5, e.g. this:

test: $.relatedObject.[?(@property === "atmosphere" && @ === "POSITIVE")]

now appears to do a recursive lookup underneath relatedObject, which could take very long if the object is large (e.g. many reportPeople w/notes etc.). Changing the lookup to:

test: $.relatedObject[?(@property === "atmosphere" && @ === "POSITIVE")]

speeds this up significantly (by a factor of 10), although it's still a lot slower than with v4 (v4: 3 ms or less, v5: more than 100 ms).

@gjvoosten
Copy link
Collaborator

although it's still a lot slower than with v4 (v4: 3 ms or less, v5: more than 100 ms)

The slowness is only apparent in the browser, it seems. I tested both v4 and v5 with https://github.com/andykais/json-querying-performance-testing and saw no significant slowdown between the two (but this is running in NodeJS, not the browser).

@gjvoosten
Copy link
Collaborator

The slowness is only apparent in the browser, it seems. I tested both v4 and v5 with https://github.com/andykais/json-querying-performance-testing and saw no significant slowdown between the two (but this is running in NodeJS, not the browser).

In the browser demo of jsonpath-plus, there is also no speed difference between v4 and v5. If I revert to ANET 2.1.39 (just before the Webpack 5 upgrade) and update jsonpath-plus, the slowness is gone. So the most likely suspect is our Webpack 5 config.

@renovate renovate bot force-pushed the renovate/jsonpath-plus-5.x branch from 7bcb5fd to 4eeeb45 Compare February 3, 2021 08:20
@renovate renovate bot force-pushed the renovate/jsonpath-plus-5.x branch from 4eeeb45 to c56c68b Compare February 3, 2021 08:23
@cemalettin-work cemalettin-work force-pushed the renovate/jsonpath-plus-5.x branch from ad8aaf7 to 0799027 Compare February 3, 2021 10:15
@gjvoosten
Copy link
Collaborator

So the most likely suspect is our Webpack 5 config.

Turns out it's due the the package exports of jsonpath-plus; see JSONPath-Plus/JSONPath#145 and JSONPath-Plus/JSONPath#146

@cemalettin-work cemalettin-work force-pushed the renovate/jsonpath-plus-5.x branch from b1f4afe to a54032b Compare February 3, 2021 14:43
…browser

Brings performance back to normal speed, so test should pass again.
@gjvoosten gjvoosten merged commit 0230046 into candidate Feb 8, 2021
@gjvoosten gjvoosten deleted the renovate/jsonpath-plus-5.x branch February 8, 2021 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants