Upgrade non-breaking JS dependencies #14152

pluma · 2021-05-06T12:57:04Z

Scope & Purpose

accepts 1.3.5 -> 1.3.7
ansi_up 4.0.3 -> 5.0.1
chalk 1.1.3 -> 4.1.1
content-type (added) -> 1.0.4
error-stack-parser 2.0.2 -> 2.0.6
highlight.js 9.15.6 -> 10.7.2
http-errors 1.7.2 -> 1.8.0
iconv-lite 0.4.24 -> 0.6.2
js-yaml 3.13.1 -> ~~4.1.0~~ 3.14.1
lodash 4.17.13 -> 4.17.21
marked -> removed
media-typer -> removed
mime-types 2.1.22 -> 2.1.30
mocha -> removed
ms (added) -> 2.1.3
netmask 1.0.6 -> 2.0.2
qs 6.7.0 -> 6.10.1
range-parser 1.2.0 -> 1.2.1
semver 6.0.0 -> 7.3.5
timezone 1.0.22 -> 1.0.23
type-is 1.6.16 -> 1.6.18
underscore 1.9.1 -> 1.13.1
xmldom 0.1.27 -> 0.6.0

Closes #14120. /cc @joerg84

💩 Bugfix (requires CHANGELOG entry)
🍕 New feature (requires CHANGELOG entry, feature documentation and release notes)
🔥 Performance improvement
🔨 Refactoring/simplification
📖 CHANGELOG entry made

Backports:

No backports required
Backports required for: (Please specify versions and link PRs)

Testing & Verification

This change is a trivial rework / code cleanup without any test coverage.
The behavior in this PR was manually tested
This change is already covered by existing tests, such as every single test that involves JS.
This PR adds tests that were used to verify all changes:
- Added new C++ Unit tests
- Added new integration tests (e.g. in shell_server / shell_server_aql)
- Added new resilience tests (only if the feature is impacted by failovers)
There are tests in an external testing repository:
I ensured this code runs with ASan / TSan or other static verification tools

Link to Jenkins PR run: TODO

Documentation

All new features should be accompanied by corresponding documentation.
Bugs and features should furthermore be documented in the CHANGELOG so that
support, end users, and other developers have a concise overview.

Added entry to Release Notes
Added a new section in the Manual
Added a new section in the HTTP API
Added Swagger examples for the HTTP API
Updated license information in LICENSES-OTHER-COMPONENTS.md for 3rd party libraries

pluma · 2021-05-06T13:01:36Z

Documentation/Scripts/exampleHeader.js


  try {
-    return hljs.highlight(lang, code).value;
+    return hljs.highlight(code, {language}).value;


I'm not sure why we're bundling highlight.js given the docs live in their own repo, I think?

The API change would be breaking but the old API is still supported with deprecation warnings.

IDK, we should ask @dothebart

Not related to your changes @pluma , but this file does contain lots of methods, variables and modules (like fs) which are not even used here.

e.g. methods: logCurlRequestPlain / logCurlRequest / curlRequest
e.g. vars: ArangoshOutput, testFunc, countErrors
e.g. modules: time, fs, examples

Fix: Include those files in our JSLINT script :)

If I had one wish it's throwing out all our jslint nonsense and just using an external eslint+prettier via npm. We could probably also do this automatically for PRs.

Yes, I bet our jslint rules aren't the best. Personally I do not care which tool we're going to use. Also adding jslint support for documentation related JS files is out of scope here. But please make sure that this will not be forgotten.

@Simran-B fyi.

@Simran-B do you have any insight what this documentation file does and why we bundle highlight.js? If this code is unused we could remove the dependency.

To me it looks like the file is used in generateExamples.py so apparently it's part of the docs build process? Seems a bit extreme to bundle and distribute JS dependencies that are only ever used once during build and then never again.

js/client/modules/@arangodb/testsuites/arangosh.js

js/common/modules/@arangodb/util.js

pluma · 2021-05-06T13:03:59Z

We still have a bunch of outdated packages but most of them are likely breaking in ways that would impact users:

Package                Current      Wanted       Latest  Location                         Depended by
chai                     3.5.0       3.5.0        4.3.4  node_modules/chai                node
eslint                  5.16.0      5.16.0       7.25.0  node_modules/eslint              node
graphql-sync        0.6.2-sync  0.6.2-sync  0.10.1-sync  node_modules/graphql-sync        node
iconv-lite              0.4.24      0.4.24        0.6.2  node_modules/iconv-lite          node
joi                     14.3.1      14.3.1       17.4.0  node_modules/joi                 node
joi-to-json-schema       4.0.1       4.0.1        5.1.0  node_modules/joi-to-json-schema  node
marked                   0.6.2       0.6.2        2.0.3  node_modules/marked              node
media-typer              0.3.0       0.3.0        1.1.0  node_modules/media-typer         node
mocha                    6.1.3       6.1.3        8.3.2  node_modules/mocha               node
semver                   6.0.0       6.0.0        7.3.5  node_modules/semver              node
sinon                   1.17.6      1.17.6       10.0.0  node_modules/sinon               node
statuses                 1.5.0       1.5.0        2.0.1  node_modules/statuses            node
xmldom                  0.1.27      0.1.27        0.6.0  node_modules/xmldom              node

pluma · 2021-05-06T13:07:21Z

@joerg84 Remind me that we need to find a way to hide the bundled modules from users in ArangoDB 4 so we can stop worrying about breaking changes as they only affect our internals. We can just wrap the stuff we want users to be able to access and tell them to bundle their own dependencies for the rest.

js/node/package.json

hkernbach

Module API change needs to be solved/discusses.
Enable JSlint for documentation related JS files.
Check if both (lodash and underscore) are officially supported and needed to be bundled.

pluma · 2021-05-11T15:04:29Z

@hkernbach marked is not listed as a bundled module and does not seem to be used in any of our code, can we remove it?

https://www.arangodb.com/docs/stable/appendix-java-script-modules.html#bundled-npm-modules

I think it used to be used in aardvark (to render the README?) but apparently isn't any longer or maybe it's now being used in the frontend instead.

Note that our version of marked is on 0.x and the latest is 2.x so upgrading it would be a breaking upgrade.

pluma · 2021-05-11T15:46:50Z

The following deps can't be upgraded as the introduce BREAKING changes to their APIs and are publicly listed:

chai 3.5.0 -> 4.3.4
graphql-sync 0.6.2-sync -> 0.10.1-sync (deprecated and should be removed in arangodb 4)
joi 14.3.1 -> 17.4.0 (also v17 doesn't run in ArangoDB right now)
js-yaml 3.14.1 -> 4.1.0
sinon 1.17.6 -> 10.0.0
statuses 1.5.0 -> 2.0.1

The following internal deps can't be upgraded without significant changes to our internal code:

eslint 5.16.0 -> 7.26.0 (used in js/common/modules/jslint.js but should be replaced with external node tooling IMO)
joi-to-json-schema 4.0.1 -> 5.1.0 (tied to joi 14)
marked 0.6.2 -> 2.0.3 (but can probably be removed entirely?)
mocha 6.1.3 -> 8.4.0 (should be replaced with our own implementation)
xmldom 0.1.27 -> 0.6.0 (used in js/client/modules/@rarangodb/testsuites/export.js)

hkernbach · 2021-05-11T16:11:40Z

@hkernbach marked is not listed as a bundled module and does not seem to be used in any of our code, can we remove it?

https://www.arangodb.com/docs/stable/appendix-java-script-modules.html#bundled-npm-modules

I think it used to be used in aardvark (to render the README?) but apparently isn't any longer or maybe it's now being used in the frontend instead.

Note that our version of marked is on 0.x and the latest is 2.x so upgrading it would be a breaking upgrade.

If it is unused + not documented (officially supported): Sure let's remove it.

pluma · 2021-06-07T12:39:39Z

Jenkins: https://jenkins.arangodb.biz/view/PR/job/arangodb-matrix-pr/15761/

pluma · 2021-07-05T08:21:10Z

@hkernbach I think tests should pass now. I had to restore underscore because some of our own services on the Foxx app store use it. There was also a legit regression caused by a dependency change that I've fixed.

pluma · 2021-07-06T14:25:47Z

I've bumped xmldom since it's only used in the export testsuite and that one still works with the new version.

accepts 1.3.5 -> 1.3.7 ansi_up 4.0.3 -> 5.0.1 chalk 1.1.3 -> 4.1.1 error-stack-parser 2.0.2 -> 2.0.6 highlight.js 9.15.6 -> 10.7.2 http-errors 1.7.2 -> 1.8.0 js-yaml 3.13.1 -> 4.1.0 lodash 4.17.13 -> 4.17.21 mime-types 2.1.22 -> 2.1.30 netmask 1.0.6 -> 2.0.2 qs 6.7.0 -> 6.10.1 range-parser 1.2.0 -> 1.2.1 timezone 1.0.22 -> 1.0.23 type-is 1.6.16 -> 1.6.18 underscore 1.9.1 -> 1.13.1 Closes #14120.

iconv-lite 0.4.24 -> 0.6.2 semver 6.0.0 -> 7.3.5 media-typer 0.3.0 -> content-type 1.0.4

But mostly it wasn't used at all so the useless imports have been removed.

Apparently some of our demo services use this, breaking tests.

This reverts commit 261df1d. This commit breaks shell-foxx-swagger-spec.js

This is used by the mocha internals and was dropped in the rebase.

This previously broke a testcase.

This is actually used in the Foxx HTTP API app.

KVS85 · 2021-07-13T13:43:01Z

Image Digest: sha256:4301596093d4986c28aecda4622e0e75f2c4302b4da65613469bd9efabd3afc5
Full Tag: gcr.io/gcr-for-testing/arangodb/arangodb-preview:PR14152
Image ID: 91e0a945ea5eb791526c1811c14e6e122a66054857d56392c000923c174dc406
Status: pass
Last Eval: 2021-07-12T22:11:44Z
Policy ID: 2c53a13c-1765-11e8-82ef-23527761d060
Final Action: warn
Final Action Reason: policy_evaluation

Gate              Trigger            Detail                                                                                     Status
dockerfile        instruction        Dockerfile directive 'HEALTHCHECK' not found, matching condition 'not_exists' check        warn

Image Digest: sha256:a4ed9f0f3a03a26f191875b8978578d552e14b441a11fd9c167f237e3d2731cd
Full Tag: gcr.io/gcr-for-testing/arangodb/enterprise-preview:PR14152
Image ID: d07953517088ac26ff9a07276305c3d494f044ca4158a79cfae214100f62555c
Status: pass
Last Eval: 2021-07-13T13:42:48Z
Policy ID: 2c53a13c-1765-11e8-82ef-23527761d060
Final Action: warn
Final Action Reason: policy_evaluation

Gate              Trigger            Detail                                                                                     Status
dockerfile        instruction        Dockerfile directive 'HEALTHCHECK' not found, matching condition 'not_exists' check        warn

pluma · 2021-07-14T13:57:33Z

CHANGELOG

@@ -338,6 +366,32 @@ devel
 * Fixed ES-881: ensure that LDAP options for async, referrals and restart set
  the off value correctly. Otherwise, this can result in an "operations error".

+* Updated JavaScript dependencies, including breaking changes to non-public


pluma added 1 Feature 3 JS API labels May 6, 2021

pluma self-assigned this May 6, 2021

pluma reviewed May 6, 2021

View changes

pluma added this to the devel milestone May 6, 2021

joerg84 requested a review from hkernbach May 7, 2021

hkernbach reviewed May 7, 2021

View changes

js/node/package.json Show resolved Hide resolved

hkernbach requested changes May 7, 2021

View changes

pluma added the DO_NOT_MERGE label May 11, 2021

pluma requested a review from hkernbach May 11, 2021

hkernbach approved these changes May 12, 2021

View changes

pluma added 9 WIP and removed DO_NOT_MERGE labels May 22, 2021

pluma removed the 9 WIP label Jul 7, 2021

pluma requested a review from ajurdak Jul 7, 2021

pluma added 7 commits May 6, 2021

Downgrade js-yaml

0c17fec

Bump deps

ee12551

iconv-lite 0.4.24 -> 0.6.2 semver 6.0.0 -> 7.3.5 media-typer 0.3.0 -> content-type 1.0.4

Add note to CHANGELOG

ffa547b

Add content-type license / remove media-typer license

b7b79b7

Replace underscore imports with lodash

ecbf83f

But mostly it wasn't used at all so the useless imports have been removed.

Remove marked, underscore

c4bd190

pluma added 20 commits Jul 4, 2021

Add underscore back

73688d6

Apparently some of our demo services use this, breaking tests.

Fix Foxx type internals

601e3ed

Bump deps

5cfcae5

npm audit fix

0f651e0

Keep mime type parameters

cc410a2

xmldom -> 0.6.0 (BREAKING)

62d27b2

Revert "Keep mime type parameters"

2f95be1

This reverts commit 261df1d. This commit breaks shell-foxx-swagger-spec.js

Extract used parts of mocha

4114ad1

Convert to classes

19ac18c

Better module load error messages

262c08f

Move mocha.js to mocha/index.js

10ee09d

Extract reporters, interfaces, utils

a12ff8e

Remove mocha dep

3ec2e73

Add mocha copyright headers

a011a16

jslint

35de614

Add ms

26a6bc9

This is used by the mocha internals and was dropped in the rebase.

Restore cause attribute

b1e8e67

This previously broke a testcase.

Fix utils/jslint.sh

d004dae

Fix test title

64b4e67

Fix missing reporters exports

74ee2c7

This is actually used in the Foxx HTTP API app.

pluma force-pushed the feature/bump-deps-2021-05 branch from 9cd5686 to 74ee2c7 Jul 12, 2021

KVS85 merged commit f98dd99 into devel Jul 13, 2021
1 check passed

1 check passed

arangodb-matrix-pr The build succeeded! Ready to be merged after valid review!
Details

KVS85 deleted the feature/bump-deps-2021-05 branch Jul 13, 2021

This was referenced Jul 14, 2021

[3.8] Upgrade non-breaking JS dependencies #14501

Merged

[3.7] Upgrade non-breaking JS dependencies #14502

Merged

[3.6] Upgrade non-breaking JS dependencies #14503

Merged

pluma reviewed Jul 14, 2021

View changes

pluma mentioned this pull request Jul 15, 2021

Remove chalk #14505

Merged

arangodb / arangodb

Upgrade non-breaking JS dependencies #14152

Upgrade non-breaking JS dependencies #14152

pluma commented May 6, 2021 •

edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

pluma commented May 6, 2021

pluma commented May 6, 2021

hkernbach left a comment

pluma commented May 11, 2021 •

edited

pluma commented May 11, 2021

hkernbach commented May 11, 2021

pluma commented Jun 7, 2021

pluma commented Jul 5, 2021

pluma commented Jul 6, 2021

KVS85 commented Jul 13, 2021

This comment has been minimized.

arangodb / arangodb

Upgrade non-breaking JS dependencies #14152

Upgrade non-breaking JS dependencies #14152

Conversation

pluma commented May 6, 2021 • edited

Scope & Purpose

Backports:

Testing & Verification

Documentation

This comment has been minimized.

pluma May 6, 2021 Author Contributor

This comment has been minimized.

hkernbach May 7, 2021 Contributor

This comment has been minimized.

hkernbach May 7, 2021 Contributor

This comment has been minimized.

hkernbach May 7, 2021 Contributor

This comment has been minimized.

pluma May 11, 2021 Author Contributor

This comment has been minimized.

hkernbach May 11, 2021 Contributor

This comment has been minimized.

pluma May 22, 2021 Author Contributor

This comment has been minimized.

pluma May 22, 2021 Author Contributor

pluma commented May 6, 2021

pluma commented May 6, 2021

hkernbach left a comment

pluma commented May 11, 2021 • edited

pluma commented May 11, 2021

hkernbach commented May 11, 2021

pluma commented Jun 7, 2021

pluma commented Jul 5, 2021

pluma commented Jul 6, 2021

KVS85 commented Jul 13, 2021

This comment has been minimized.

pluma Jul 14, 2021 Author Contributor

pluma commented May 6, 2021 •

edited

pluma May 6, 2021
Author Contributor

hkernbach May 7, 2021
Contributor

hkernbach May 7, 2021
Contributor

hkernbach May 7, 2021
Contributor

pluma May 11, 2021
Author Contributor

hkernbach May 11, 2021
Contributor

pluma May 22, 2021
Author Contributor

pluma May 22, 2021
Author Contributor

pluma commented May 11, 2021 •

edited

pluma Jul 14, 2021
Author Contributor