Skip to content

Conversation

@andrewbranch
Copy link
Member

Fixing breakages from microsoft/TypeScript#62338

Not Fixed

babel-plugin-glaze

Typings for glaze are broken when respecting package.json exports. The last publish was 5+ years ago.

Fixed

array-normalize, fs-extra, hot-formula-parser

These were dual ESM/CJS packages using a bit of a hack to test the ESM and CJS entrypoints, which broke when respecting package.json exports. Converted them to --module node16 and fixed test imports.

cypress-dotenv, dotenv-safe

These had a dependency on an old version of dotenv that has typings that are broken when respecting package.json exports. In reality, the dependency should have been a peerDependency. Updated their self versions, moved dependency to peerDependency with version matching the implementation package, and added a devDependency version to ensure the one installed for testing has working types.

dotenv-defaults

This one has a true dependency on dotenv, but the latest version of dotenv-defaults requires a dotenv version greater than the one where typings were fixed. Updated the self version and dependency version.

expect-puppeteer

Tests exercised await import, which behaves differently with/without esModuleInterop, which is implied by bundler. Set esModuleInterop to true in tsconfig.

ltx

Typings intended to reflect a dual ESM/CJS package but wasn’t quite right. Had to move a lot of stuff around and use nested package.json files, which I'm not sure DT-tools can handle currently, but we should add support if needed, because there's no other way to do this correctly.

@Renegade334
Copy link
Contributor

Regarding ltx, what's the difference in behaviour between just keeping the entire package as de facto CJS as it is currently, versus subtyping specific paths as type: module? Does the compiler look to the @types package structure to work out what import pattern it needs to emit when targeting the JS module?

@andrewbranch
Copy link
Member Author

The first change I had to make to make the tests compile was adding the "./src/*" and "./lib/*" exports mappings that the implementation package has. Since those are exposed in the public API (I didn’t check whether they are documented as public, but by merit of being in exports and the tests importing them, I consider them public), it means that someone in CJS could now do

import parsers = require("ltx/src/parsers.js")

and if those files are not correctly marked as ESM, that require would be allowed in --module node16 and node18, where it should fail due to require(esm) being disallowed.

@andrewbranch
Copy link
Member Author

Where is @typescript-bot? 🤔

@jakebailey
Copy link
Member

The bot does not respond to PRs sent from the repo itself, only forks. It's a bug. I'm going to quick capture a test to understand why it's breaking before I forget.

@jakebailey
Copy link
Member

The test implies it should be replying, so it's possibly some other permission problem with the token...

@jakebailey
Copy link
Member

@typescript-bot hello

@typescript-bot
Copy link
Contributor

typescript-bot commented Aug 29, 2025

@andrewbranch Thank you for submitting this PR!

This is a live comment that I will keep updated.

9 packages in this PR

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

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

Status

  • ✅ No merge conflicts
  • ✅ Continuous integration tests have passed
  • ✅ A DT maintainer needs to approve changes that affect more than one package

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": 73574,
  "author": "andrewbranch",
  "headCommitOid": "ff86211ca7d617c4758d28f5221d4043a9576bf5",
  "mergeBaseOid": "fe9f2bfc7c6569a075c68ed4cb1a1e0a3d900d00",
  "lastPushDate": "2025-08-28T22:10:13.000Z",
  "lastActivityDate": "2025-09-10T16:14:51.000Z",
  "mergeOfferDate": "2025-09-10T16:11:20.000Z",
  "mergeRequestDate": "2025-09-10T16:14:51.000Z",
  "mergeRequestUser": "andrewbranch",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "array-normalize",
      "kind": "edit",
      "files": [
        {
          "path": "types/array-normalize/array-normalize-tests.mts",
          "kind": "test"
        },
        {
          "path": "types/array-normalize/tsconfig.json",
          "kind": "package-meta-ok"
        }
      ],
      "owners": [
        "BendingBender"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "babel-plugin-glaze",
      "kind": "delete",
      "files": [
        {
          "path": "types/babel-plugin-glaze/.npmignore",
          "kind": "package-meta-ok"
        },
        {
          "path": "types/babel-plugin-glaze/babel-plugin-glaze-tests.tsx",
          "kind": "test"
        },
        {
          "path": "types/babel-plugin-glaze/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/babel-plugin-glaze/package.json",
          "kind": "package-meta-ok"
        },
        {
          "path": "types/babel-plugin-glaze/tsconfig.json",
          "kind": "package-meta-ok"
        }
      ],
      "owners": [
        "kripod"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "cypress-dotenv",
      "kind": "edit",
      "files": [
        {
          "path": "types/cypress-dotenv/package.json",
          "kind": "package-meta",
          "suspect": "not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-packagejson) and not moving towards it (check: `peerDependencies`)"
        }
      ],
      "owners": [
        "daikiojm",
        "peterblazejewicz"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "dotenv-defaults",
      "kind": "edit",
      "files": [
        {
          "path": "types/dotenv-defaults/package.json",
          "kind": "package-meta-ok"
        }
      ],
      "owners": [
        "peterblazejewicz"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "dotenv-safe",
      "kind": "edit",
      "files": [
        {
          "path": "types/dotenv-safe/package.json",
          "kind": "package-meta",
          "suspect": "not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-packagejson) and not moving towards it (check: `peerDependencies`)"
        }
      ],
      "owners": [
        "krenor"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    },
    {
      "name": "expect-puppeteer",
      "kind": "edit",
      "files": [
        {
          "path": "types/expect-puppeteer/expect-puppeteer-tests.ts",
          "kind": "test"
        },
        {
          "path": "types/expect-puppeteer/tsconfig.json",
          "kind": "package-meta",
          "suspect": "not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-tsconfigjson) and not moving towards it (check: `compilerOptions.esModuleInterop`)"
        }
      ],
      "owners": [
        "tkrotoff",
        "jfm710"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "fs-extra",
      "kind": "edit",
      "files": [
        {
          "path": "types/fs-extra/test/fs-extra-tests-module.mts",
          "kind": "test"
        },
        {
          "path": "types/fs-extra/tsconfig.json",
          "kind": "package-meta-ok"
        }
      ],
      "owners": [
        "alan-agius4",
        "midknight41",
        "shiftkey",
        "mees-",
        "jrockwood",
        "sangdth",
        "ffflorian",
        "peterblazejewicz",
        "NotWoods",
        "BendingBender"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    },
    {
      "name": "hot-formula-parser",
      "kind": "edit",
      "files": [
        {
          "path": "types/hot-formula-parser/hot-formula-parser-tests.mts",
          "kind": "test"
        },
        {
          "path": "types/hot-formula-parser/tsconfig.json",
          "kind": "package-meta-ok"
        }
      ],
      "owners": [
        "joao-mbn"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "ltx",
      "kind": "edit",
      "files": [
        {
          "path": "types/ltx/ltx-tests.ts",
          "kind": "test"
        },
        {
          "path": "types/ltx/package.json",
          "kind": "package-meta-ok"
        }
      ],
      "owners": [
        "BendingBender"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "jakebailey",
      "date": "2025-09-10T16:10:36.000Z",
      "isMaintainer": true
    }
  ],
  "mainBotCommentID": 3235348553,
  "ciResult": "pass"
}

@typescript-bot
Copy link
Contributor

typescript-bot commented Aug 29, 2025

🔔 @BendingBender @kripod @daikiojm @peterblazejewicz @krenor @tkrotoff @jfm710 @alan-agius4 @midknight41 @shiftkey @mees- @jrockwood @sangdth @ffflorian @NotWoods @joao-mbn — 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 typescript-bot moved this to Needs Maintainer Review in Pull Request Status Board Aug 29, 2025
Copy link
Member

@jakebailey jakebailey Aug 29, 2025

Choose a reason for hiding this comment

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

dt-tools definitely can't handle this one; this will be skipped entirely in the built package, and probably break linting?

Copy link
Member

Choose a reason for hiding this comment

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

Break linting as in, it will probably think these are script files (like we have a sub package in the node dirs)

@jakebailey
Copy link
Member

I would honestly just delete babel-plugin-glaze, babel-plugin-glaze hasn't been updated in 5 years either, so if it's blocking us, oh well.

@Renegade334
Copy link
Contributor

It looks like ltx has actually just removed the CJS side of the package (xmppjs/ltx#160), it just hasn't been published yet.

Is the easiest thing just to wait for this to hit a published version, and then update DT accordingly, rather than necessitating a change to the tooling?

@Renegade334
Copy link
Contributor

Alternatively, with that being the case, would it be easier to make a "legal fiction" change now, and fix the exports map while still assuming a CJS module (as the types do currently), since the package can be updated to a proper type: module soon anyway when that change is published?

It feels like chasing the edge case (submodule exports from a package that uses nested package.json stubs to change the module type of specific subpaths) for one specific and soon-not-to-exist example probably isn't worth the effort?

@andrewbranch
Copy link
Member Author

Yeah, I can probably find a less disruptive way to make ltx compile.

@typescript-bot typescript-bot added the Unreviewed No one showed up to review this PR, so it'll be reviewed by a DT maintainer. label Sep 9, 2025
@typescript-bot
Copy link
Contributor

typescript-bot commented Sep 9, 2025

Re-ping @BendingBender, @kripod, @daikiojm, @peterblazejewicz, @krenor, @tkrotoff, @jfm710, @alan-agius4, @midknight41, @shiftkey, @mees-, @jrockwood, @sangdth, @ffflorian, @NotWoods, @joao-mbn:

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 typescript-bot moved this from Needs Maintainer Review to Waiting for Code Reviews in Pull Request Status Board Sep 10, 2025
@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in Pull Request Status Board Sep 10, 2025
@typescript-bot
Copy link
Contributor

@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 typescript-bot added Maintainer Approved Self Merge This PR can now be self-merged by the PR author or an owner and removed Unreviewed No one showed up to review this PR, so it'll be reviewed by a DT maintainer. labels Sep 10, 2025
@typescript-bot typescript-bot moved this from Needs Maintainer Review to Waiting for Author to Merge in Pull Request Status Board Sep 10, 2025
@typescript-bot
Copy link
Contributor

@andrewbranch: Everything looks good here. I am ready to merge this PR (at ff86211) 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! ❤️

@andrewbranch
Copy link
Member Author

Ready to merge

@typescript-bot typescript-bot moved this from Waiting for Author to Merge to Recently Merged in Pull Request Status Board Sep 10, 2025
@typescript-bot typescript-bot merged commit 3ac9530 into master Sep 10, 2025
4 checks passed
@typescript-bot typescript-bot deleted the node10-deprecation-fixes branch September 10, 2025 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Check Config Changes a module config files Critical package Edits multiple packages Maintainer Approved Self Merge This PR can now be self-merged by the PR author or an owner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants