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: correct dev-deps required for the recent eslint@8 upgrade #3449

Merged
merged 5 commits into from Jul 4, 2023

Conversation

trentm
Copy link
Member

@trentm trentm commented Jun 27, 2023

The upgrade to eslint@8 in #3409 subtly broken peer-dependencies.
npm ls unhelpfully does not complain, but npm ls -a shows some
errors, and ./dev-utils/make-distribution.sh was broken.

The upgrade bumped "eslint-config-standard" from ^14.1.1 to ^16.
However, v16 has:

    "peerDependencies": {
      "eslint": "^7.12.1",
      "eslint-plugin-import": "^2.22.1",
      "eslint-plugin-node": "^11.1.0",
      "eslint-plugin-promise": "^4.2.1 || ^5.0.0"
    },

so is incompatible with eslint@8. We need eslint-config-standard@17,
which has:

    "peerDependencies": {
      "eslint": "^8.0.1",
      "eslint-plugin-import": "^2.25.2",
      "eslint-plugin-n": "^15.0.0 || ^16.0.0 ",
      "eslint-plugin-promise": "^6.0.0"
    },

Note that also requires a change from eslint-plugin-node (unmaintained)
to the replacement "eslint-plugin-n".

The upgrade to eslint@8 in #3409 subtly broken peer-dependencies.
`npm ls` unhelpfully does not complain, but `npm ls -a` shows some
errors, and `./dev-utils/make-distribution.sh` was broken.

The upgrade bumped "eslint-config-standard" from ^14.1.1 to ^16.
However, v16 has:
    "peerDependencies": {
      "eslint": "^7.12.1",
      "eslint-plugin-import": "^2.22.1",
      "eslint-plugin-node": "^11.1.0",
      "eslint-plugin-promise": "^4.2.1 || ^5.0.0"
    },
so is incompatible with eslint@8. We need eslint-config-standard@17,
which has:
    "peerDependencies": {
      "eslint": "^8.0.1",
      "eslint-plugin-import": "^2.25.2",
      "eslint-plugin-n": "^15.0.0 || ^16.0.0 ",
      "eslint-plugin-promise": "^6.0.0"
    },

Note that also requires a change from eslint-plugin-node (unmaintained)
to the replacement "eslint-plugin-n".
@trentm trentm requested a review from david-luna June 27, 2023 15:34
@trentm trentm self-assigned this Jun 27, 2023
@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label Jun 27, 2023
@elastic-apm-tech elastic-apm-tech added this to In Progress in APM-Agents (OLD) Jun 27, 2023
@trentm
Copy link
Member Author

trentm commented Jun 27, 2023

The errors that npm ls -a showed:

$ npm ls -a
...
├─┬ eslint-config-standard@16.0.3
│ ├── eslint-plugin-import@2.27.5 deduped
│ ├── eslint-plugin-node@11.1.0 deduped
│ ├── eslint-plugin-promise@6.1.1 deduped invalid: "^4.2.1 || ^5.0.0" from node_modules/eslint-config-standard
│ └── eslint@8.42.0 deduped invalid: "^7.12.1" from node_modules/eslint-config-standard
..
│ ├── eslint@8.42.0 deduped invalid: "^7.12.1" from node_modules/eslint-config-standard
...
├─┬ eslint-plugin-node@11.1.0
│ ├─┬ eslint-plugin-es@3.0.1
│ │ ├─┬ eslint-utils@2.1.0
│ │ │ └── eslint-visitor-keys@1.3.0 deduped
│ │ ├── eslint@8.42.0 deduped invalid: "^7.12.1" from node_modules/eslint-config-standard
│ │ └── regexpp@3.2.0
│ ├─┬ eslint-utils@2.1.0
│ │ └── eslint-visitor-keys@1.3.0
│ ├── eslint@8.42.0 deduped invalid: "^7.12.1" from node_modules/eslint-config-standard
│ ├── ignore@5.2.0
│ ├── minimatch@3.1.2 deduped
│ ├── resolve@1.22.2 deduped
│ └── semver@6.3.0 deduped
├─┬ eslint-plugin-promise@6.1.1 invalid: "^4.2.1 || ^5.0.0" from node_modules/eslint-config-standard
│ └── eslint@8.42.0 deduped invalid: "^7.12.1" from node_modules/eslint-config-standard
├─┬ eslint@8.42.0 invalid: "^7.12.1" from node_modules/eslint-config-standard
│ ├─┬ @eslint-community/eslint-utils@4.4.0
│ │ ├── eslint-visitor-keys@3.4.1
│ │ └── eslint@8.42.0 deduped invalid: "^7.12.1" from node_modules/eslint-config-standard
...

The error in make-distribution.sh (which would have showed up when attempting to do a release):

% ./dev-utils/make-distribution.sh
elastic-apm-node-3.47.0.tgz
npm ERR! code ERESOLVE
npm ERR! ERESOLVE could not resolve
npm ERR!
npm ERR! While resolving: eslint-config-standard@16.0.3
npm ERR! Found: eslint@8.42.0
npm ERR! node_modules/eslint
npm ERR!   dev eslint@"^8.42.0" from the root project
npm ERR!   peer eslint@"^6.0.0 || ^7.0.0 || >=8.0.0" from @eslint-community/eslint-utils@4.4.0
npm ERR!   node_modules/@eslint-community/eslint-utils
npm ERR!     @eslint-community/eslint-utils@"^4.2.0" from eslint@8.42.0
npm ERR!   5 more (eslint-plugin-es, eslint-plugin-import, ...)
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer eslint@"^7.12.1" from eslint-config-standard@16.0.3
npm ERR! node_modules/eslint-config-standard
npm ERR!   dev eslint-config-standard@"^16" from the root project
npm ERR!
npm ERR! Conflicting peer dependency: eslint@7.32.0
npm ERR! node_modules/eslint
npm ERR!   peer eslint@"^7.12.1" from eslint-config-standard@16.0.3
npm ERR!   node_modules/eslint-config-standard
npm ERR!     dev eslint-config-standard@"^16" from the root project
npm ERR!
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
npm ERR!
npm ERR! See /Users/trentm/.npm/eresolve-report.txt for a full report.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/trentm/.npm/_logs/2023-06-27T00_39_06_792Z-debug-0.log

@trentm trentm changed the title fix: correct dev-deps required for the recent estlin@8 upgrade fix: correct dev-deps required for the recent estlint@8 upgrade Jun 27, 2023
@trentm trentm changed the title fix: correct dev-deps required for the recent estlint@8 upgrade fix: correct dev-deps required for the recent eslint@8 upgrade Jun 27, 2023
@trentm
Copy link
Member Author

trentm commented Jun 27, 2023

Lovely. These dev-dep updates change the eslint rules in play, so now lint fails:

lint failures
% make check
npm run lint

> elastic-apm-node@3.47.0 lint
> npm run lint:eslint && npm run lint:license-files && npm run lint:yaml-files && npm run lint:tav


> elastic-apm-node@3.47.0 lint:eslint
> eslint . # requires node >12.20.0


/Users/trentm/el/apm-agent-nodejs2/dev-utils/bitrot.js
  354:40  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/lib/agent.js
  609:7  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/lib/http-request.js
  210:5  warning  Expected property shorthand  object-shorthand
  211:5  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/lib/instrumentation/dropped-spans-stats.js
  63:7  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/lib/instrumentation/http-shared.js
  102:13  warning  Expected property shorthand  object-shorthand
  189:80  warning  Expected property shorthand  object-shorthand
  264:51  warning  Expected property shorthand  object-shorthand
  264:95  warning  Expected property shorthand  object-shorthand
  283:77  warning  Expected property shorthand  object-shorthand
  291:79  warning  Expected property shorthand  object-shorthand
  322:82  warning  Expected property shorthand  object-shorthand
  326:81  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/lib/instrumentation/modules/@aws-sdk/client-s3.js
  195:11  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/lib/instrumentation/modules/elasticsearch.js
   84:96   warning  Expected property shorthand  object-shorthand
   84:104  warning  Expected property shorthand  object-shorthand
   84:120  warning  Expected property shorthand  object-shorthand
  148:100  warning  Expected property shorthand  object-shorthand
  148:108  warning  Expected property shorthand  object-shorthand
  148:124  warning  Expected property shorthand  object-shorthand
  158:79   warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/lib/instrumentation/modules/generic-pool.js
  20:74   warning  Expected property shorthand  object-shorthand
  35:73   warning  Expected property shorthand  object-shorthand
  62:101  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/lib/instrumentation/modules/graphql.js
  205:14  warning  Expected property shorthand  object-shorthand
  205:32  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/lib/instrumentation/modules/http2.js
  112:11  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/lib/instrumentation/modules/mongodb-core.js
   48:92   warning  Expected property shorthand  object-shorthand
   48:100  warning  Expected property shorthand  object-shorthand
   72:95   warning  Expected property shorthand  object-shorthand
   89:93   warning  Expected property shorthand  object-shorthand
   89:101  warning  Expected property shorthand  object-shorthand
  106:96   warning  Expected property shorthand  object-shorthand
  122:93   warning  Expected property shorthand  object-shorthand
  146:96   warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/lib/instrumentation/modules/mysql.js
  84:60   error  Expected error to be handled                                  n/handle-callback-err
  84:104  error  Definition for rule 'node/handle-callback-err' was not found  node/handle-callback-err

/Users/trentm/el/apm-agent-nodejs2/lib/instrumentation/modules/redis.js
   82:28  warning  Expected property shorthand  object-shorthand
  115:28  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/lib/instrumentation/patch-async.js
  306:48  error  Definition for rule 'node/no-deprecated-api' was not found  node/no-deprecated-api
  307:7   error  'fs.lchmod' was deprecated since v0.4.0                     n/no-deprecated-api
  307:48  error  Definition for rule 'node/no-deprecated-api' was not found  node/no-deprecated-api

/Users/trentm/el/apm-agent-nodejs2/lib/instrumentation/template-shared.js
  16:9  warning  Expected property shorthand  object-shorthand
  17:9  warning  Expected property shorthand  object-shorthand
  33:9  warning  Expected property shorthand  object-shorthand
  34:9  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/lib/instrumentation/transaction.js
  123:123  warning  Expected property shorthand  object-shorthand
  139:125  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/lib/lambda.js
  394:7  warning  Expected property shorthand  object-shorthand
  628:9  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/lib/logging.js
  132:9  warning  Expected property shorthand  object-shorthand
  153:5  warning  Expected property shorthand  object-shorthand
  178:3  warning  Expected property shorthand  object-shorthand
  179:3  warning  Expected property shorthand  object-shorthand
  180:3  warning  Expected property shorthand  object-shorthand
  181:3  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/lib/stacktraces.js
  309:19  warning  Expected property shorthand  object-shorthand
  319:21  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/_mock_apm_server.js
  98:9  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/benchmarks/utils/bench.js
  44:9  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/cloud-metadata/test-server.test.js
   50:7  warning  Expected property shorthand  object-shorthand
   72:7  warning  Expected property shorthand  object-shorthand
  104:9  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/instrumentation/azure-functions/fixtures/AJsAzureFnApp/HttpFnDistTraceA/index.js
  28:11  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/instrumentation/express-utils.test.js
  31:5  warning  Expected property shorthand  object-shorthand
  33:7  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/instrumentation/modules/@elastic/elasticsearch.test.js
  178:7  warning  Expected property shorthand  object-shorthand
  249:7  warning  Expected property shorthand  object-shorthand
  332:7  warning  Expected property shorthand  object-shorthand
  966:7  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/instrumentation/modules/apollo-server-express.test.js
   67:9  warning  Expected property shorthand  object-shorthand
  113:9  warning  Expected property shorthand  object-shorthand
  158:9  warning  Expected property shorthand  object-shorthand
  208:9  warning  Expected property shorthand  object-shorthand
  273:9  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/instrumentation/modules/aws-sdk/dynamodb.test.js
  168:11  warning  Expected property shorthand  object-shorthand
  217:11  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/instrumentation/modules/aws-sdk/sns.test.js
  258:11  warning  Expected property shorthand  object-shorthand
  326:11  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/instrumentation/modules/elasticsearch.test.js
   40:43  warning  Expected property shorthand  object-shorthand
   55:43  warning  Expected property shorthand  object-shorthand
   71:43  warning  Expected property shorthand  object-shorthand
   87:43  warning  Expected property shorthand  object-shorthand
  124:45  warning  Expected property shorthand  object-shorthand
  154:45  warning  Expected property shorthand  object-shorthand
  187:45  warning  Expected property shorthand  object-shorthand
  203:43  warning  Expected property shorthand  object-shorthand
  257:30  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/instrumentation/modules/express-queue.test.js
  68:7  warning  Expected property shorthand  object-shorthand
  69:7  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/instrumentation/modules/graphql.test.js
  214:7  warning  Expected property shorthand  object-shorthand
  216:7  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/instrumentation/modules/http/aborted-requests-enabled.test.js
   45:24  error  Expected error to be handled                                  n/handle-callback-err
   45:47  error  Definition for rule 'node/handle-callback-err' was not found  node/handle-callback-err
  147:24  error  Expected error to be handled                                  n/handle-callback-err
  147:47  error  Definition for rule 'node/handle-callback-err' was not found  node/handle-callback-err
  225:24  error  Expected error to be handled                                  n/handle-callback-err
  225:47  error  Definition for rule 'node/handle-callback-err' was not found  node/handle-callback-err
  314:24  error  Expected error to be handled                                  n/handle-callback-err
  314:47  error  Definition for rule 'node/handle-callback-err' was not found  node/handle-callback-err
  394:24  error  Expected error to be handled                                  n/handle-callback-err
  394:47  error  Definition for rule 'node/handle-callback-err' was not found  node/handle-callback-err
  432:24  error  Expected error to be handled                                  n/handle-callback-err
  432:47  error  Definition for rule 'node/handle-callback-err' was not found  node/handle-callback-err

/Users/trentm/el/apm-agent-nodejs2/test/instrumentation/modules/http/basic.test.js
  130:7  warning  Expected property shorthand  object-shorthand
  133:7  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/instrumentation/modules/http/github-179.test.js
  28:7  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/instrumentation/modules/http/ignoring.test.js
  147:7  warning  Expected property shorthand  object-shorthand
  148:7  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/instrumentation/modules/http/sse.test.js
  105:20  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/instrumentation/modules/http2.test.js
  470:7  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/instrumentation/modules/mysql/mysql.test.js
   91:29  warning  Expected property shorthand  object-shorthand
  104:29  warning  Expected property shorthand  object-shorthand
  195:42  warning  Expected property shorthand  object-shorthand
  209:42  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/opentelemetry-bridge/fixtures/createSpan-returns-null.js
  53:7  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/stacktraces/stacktraces.test.js
  296:7   warning  Expected property shorthand  object-shorthand
  390:11  warning  Expected property shorthand  object-shorthand

/Users/trentm/el/apm-agent-nodejs2/test/transaction-sampling.test.js
  39:7  warning  Expected property shorthand  object-shorthand

✖ 119 problems (17 errors, 102 warnings)
  0 errors and 102 warnings potentially fixable with the `--fix` option.

make: *** [check] Error 1

@trentm
Copy link
Member Author

trentm commented Jun 27, 2023

eslint-config-standard@17 changes: https://github.com/standard/standard/blob/master/CHANGELOG.md#1700---2022-04-20

@trentm trentm merged commit 3000188 into main Jul 4, 2023
36 checks passed
APM-Agents (OLD) automation moved this from In Progress to Done Jul 4, 2023
@trentm trentm deleted the trentm/fix-make-dist branch July 4, 2023 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning.
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants