[plotly.js] Update types for x and y axis names to match documentation

[mrtnbrst]

Please fill in this template.

• [x] Use a meaningful title for the pull request. Include the name of the package modified.
• [ ] Test the change in your own code. (Compile and run.)
• [x] Add or edit tests to reflect the change.
• [x] Follow the advice from the readme.
• [x] Avoid common mistakes.
• [ ] Run pnpm test <package to test>.

I did not run the tests as the command failed even before running the test

Select one of these and delete the others:

If changing an existing definition:

• [x] Provide a URL to documentation or source code which provides context for the suggested changes: https://plotly.com/python/reference/layout/shapes/#layout-shapes-items-shape-xref, https://plotly.com/python/reference/layout/shapes/#layout-shapes-items-shape-yref
• [ ] If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the package.json.

I added myself to the contributors as I contributed changes to the plotly.js type definitions three times by now and I will continue to do so once I face an error.

[typescript-bot]

@mrtnbrst Thank you for submitting this PR!

This is a live comment that I will keep updated.

1 package in this PR

• plotly.js — on npm, on unpkg
  • 1 added owner: ✎@mrtnbrst

@mrtnbrst: I see that you have added yourself as an owner, are you sure you want to become an owner?

Code Reviews

Because you edited one package and updated the tests (👏), I can help you merge this PR once someone else signs off on it.

You can test the changes of this PR in the Playground.

Status

• ✅ No merge conflicts
• ✅ Continuous integration tests have passed
• ✅ Most recent commit is approved by type definition owners or DT maintainers

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.

---

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 69871,
  "author": "mrtnbrst",
  "headCommitOid": "ea968719f26f970f4d05bf4aa9c375f5b1fc182b",
  "mergeBaseOid": "ac03da650c909c9fe0bf044f92efc58cfe930772",
  "lastPushDate": "2024-06-21T15:01:05.000Z",
  "lastActivityDate": "2024-07-29T15:57:29.000Z",
  "mergeOfferDate": "2024-07-26T22:23:29.000Z",
  "mergeRequestDate": "2024-07-29T15:57:29.000Z",
  "mergeRequestUser": "mrtnbrst",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Popular",
  "pkgInfo": [
    {
      "name": "plotly.js",
      "kind": "edit",
      "files": [
        {
          "path": "types/plotly.js/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/plotly.js/package.json",
          "kind": "package-meta-ok"
        },
        {
          "path": "types/plotly.js/test/index-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "chrisgervang",
        "martinduparc",
        "frederikaalund",
        "taoqf",
        "Dadstart",
        "szechyjs",
        "soorajpudiyadath",
        "jonfreedman",
        "meganrm",
        "milesjos",
        "skippercool",
        "mtadams007",
        "marnett-git",
        "peterblazejewicz",
        "brammitch",
        "blizzardjessica",
        "olegshilov",
        "PabloGracia",
        "jvgogh",
        "jpabdou"
      ],
      "addedOwners": [
        "mrtnbrst"
      ],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "sandersn",
      "date": "2024-07-26T22:22:51.000Z",
      "isMaintainer": true
    },
    {
      "type": "stale",
      "reviewer": "jakebailey",
      "date": "2024-07-15T22:37:39.000Z",
      "abbrOid": "75b3aec"
    }
  ],
  "mainBotCommentID": 2182922565,
  "ciResult": "pass"
}

[typescript-bot]

🔔 @chrisgervang @martinduparc @frederikaalund @taoqf @Dadstart @szechyjs @soorajpudiyadath @jonfreedman @meganrm @milesjos @skippercool @mtadams007 @marnett-git @peterblazejewicz @brammitch @blizzardjessica @olegshilov @PabloGracia @jvgogh @jpabdou — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

[typescript-bot]

@mrtnbrst Unfortunately, this pull request currently has a merge conflict 😥. Please update your PR branch to be up-to-date with respect to master. Have a nice day!

[mrtnbrst]

As mentioned above: I did not run the tests as the command failed even before running the test

[typescript-bot]

Re-ping @chrisgervang, @martinduparc, @frederikaalund, @taoqf, @Dadstart, @szechyjs, @soorajpudiyadath, @jonfreedman, @meganrm, @milesjos, @skippercool, @mtadams007, @marnett-git, @peterblazejewicz, @brammitch, @blizzardjessica, @olegshilov, @PabloGracia, @jvgogh, @jpabdou:

This PR has been out for over a week, yet I haven't seen any reviews.

Could someone please give it some attention? Thanks!

[typescript-bot]

It has been more than two weeks and this PR still has no reviews.

I'll bump it to the DT maintainer queue. Thank you for your patience, @mrtnbrst.

(Ping @chrisgervang, @martinduparc, @frederikaalund, @taoqf, @Dadstart, @szechyjs, @soorajpudiyadath, @jonfreedman, @meganrm, @milesjos, @skippercool, @mtadams007, @marnett-git, @peterblazejewicz, @brammitch, @blizzardjessica, @olegshilov, @PabloGracia, @jvgogh, @jpabdou.)

[jakebailey]

I did not run the tests as the command failed even before running the test

As mentioned above: I did not run the tests as the command failed even before running the test

I'm not sure you've posted enough info for someone to help with that; if you do, I can tell you where things are going wrong.

[mrtnbrst]

I did not run the tests as the command failed even before running the test

As mentioned above: I did not run the tests as the command failed even before running the test

I'm not sure you've posted enough info for someone to help with that; if you do, I can tell you where things are going wrong.

Sorry for my late reply - I'll post more details by the end of the week.

[typescript-bot]

@jakebailey Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

[typescript-bot]

@mrtnbrst: Everything looks good here. I am ready to merge this PR (at ea96871) on your behalf whenever you think it's ready.

If you'd like that to happen, please post a comment saying:

Ready to merge

and I'll merge this PR almost instantly. Thanks for helping out! :heart:

(@chrisgervang, @martinduparc, @frederikaalund, @taoqf, @Dadstart, @szechyjs, @soorajpudiyadath, @jonfreedman, @meganrm, @milesjos, @skippercool, @mtadams007, @marnett-git, @peterblazejewicz, @brammitch, @blizzardjessica, @olegshilov, @PabloGracia, @jvgogh, @jpabdou: you can do this too.)

[mrtnbrst]

@jakebailey Here are some more details about my failed testing.

` node ./scripts/clean-node-modules.js

pnpm install --filter plotly.js

pnpm test plotly.js`

This leads to:

OS: Windows 10, node version 22.3

pnpm test plotly.js

[email protected] test C:\Users\Admin\Documents\vscodeprojects\DefinitelyTyped node --enable-source-maps node_modules/@definitelytyped/dtslint/ types "plotly.js"

node:internal/modules/cjs/loader:1215 throw err; ^

Error: Cannot find module 'C:\Users\Admin\Documents\vscodeprojects\DefinitelyTyped\node_modules@definitelytyped\dtslint' at Module._resolveFilename (node:internal/modules/cjs/loader:1212:15) at Module._load (node:internal/modules/cjs/loader:1038:27) at wrapModuleLoad (node:internal/modules/cjs/loader:212:19) at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:158:5) at node:internal/main/run_main_module:30:49 { code: 'MODULE_NOT_FOUND', requireStack: [] }

Node.js v22.3.0  ELIFECYCLE  Test failed. See above for more details.  WARN  Local package.json exists, but node_modules missing, did you mean to install?

C:\Users\Admin\Documents\vscodeprojects\DefinitelyTyped>pnpm install --filter plotly.js Done in 14s

C:\Users\Admin\Documents\vscodeprojects\DefinitelyTyped>pnpm install --filter plotly.js Done in 15.9s

C:\Users\Admin\Documents\vscodeprojects\DefinitelyTyped>pnpm test plotly.js

[email protected] test C:\Users\Admin\Documents\vscodeprojects\DefinitelyTyped node --enable-source-maps node_modules/@definitelytyped/dtslint/ types "plotly.js"

node:internal/modules/cjs/loader:1215 throw err; ^

Error: Cannot find module 'C:\Users\Admin\Documents\vscodeprojects\DefinitelyTyped\node_modules@definitelytyped\dtslint' at Module._resolveFilename (node:internal/modules/cjs/loader:1212:15) at Module._load (node:internal/modules/cjs/loader:1038:27) at wrapModuleLoad (node:internal/modules/cjs/loader:212:19) at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:158:5) at node:internal/main/run_main_module:30:49 { code: 'MODULE_NOT_FOUND', requireStack: [] }

Node.js v22.3.0  ELIFECYCLE  Test failed. See above for more details.  WARN  Local package.json exists, but node_modules missing, did you mean to install?

Ok, then I installed dslint manually using npm install dslint

pnpm test plotly.js does not throw an error then, but it apparently does not actually run the tests. ` pnpm test plotly.js

[email protected] test C:\Users\Admin\Documents\vscodeprojects\DefinitelyTyped node --enable-source-maps node_modules/@definitelytyped/dtslint/ types "plotly.js"

[email protected] `

A throw new Error() in the beginning of a test (e.g. https://github.com/mrtnbrst/DefinitelyTyped/blob/ac03da650c909c9fe0bf044f92efc58cfe930772/types/plotly.js/test/index-tests.ts#L12) does not show any effect 😞.

What step of the README did i get wrong?

Thanks for you support and have a nice weekend!

[sandersn]

dtslint is not running the test code, only compiling it. Compile errors will fails the tests.

Confusingly, dtslint is the old package. @definitelytyped/dtslint is the new one. pnpm -w install should install packages for the whole DT repo. (Or pnpm install at the DT root.)

[mrtnbrst]

I assumed it would run the code.

I am supposed to run it README:

Create types/foo/index.d.ts containing declarations for the module "foo". You should now be able to import from "foo" in your code and it will route to the new type definition. Then build and run the code to make sure your type definition actually corresponds to what happens at runtime.

How am I supposed to run the code then? On second thought I think especially for plotly.js it does not properly work with node.js and requires a browser.

@sandersn as there are no compile errors, I assume that I can actually merge my changes?

[mrtnbrst]

@sandersn should I open an issue so someone can mark https://www.npmjs.com/package/dtslint as deprecated?

https://docs.npmjs.com/deprecating-and-undeprecating-packages-or-package-versions <-- this could ask the user to install the proper version and thereby avoid my mistake.

[sandersn]

The README is talking about running the code in your local project where you might be developing the types, not in the DT test harness. Looking at it again, it is certainly not clear about that.

With no compile errors, you can merge, yes.

I brought up dtslint deprecation with the team and we'll discuss it. The biggest obstacle is that some people use dtslint, and @definitelytyped/dtslint is not a drop-in replacement—it's even more entwined with DT infrastructure, such that it's unusable outside DT.

[mrtnbrst]

Ready to merge