@reosablo Thank you for submitting this PR! I see this is your first time submitting to DefinitelyTyped 👋 — I'm the local bot who will help you through the process of getting things through.

This is a live comment which I will keep updated.

1 package in this PR

• twitter-text — on npm, on unpkg
  • Config files to check:
    • twitter-text/OTHER_FILES.txt: not the expected form
    • twitter-text/index.d.mts: edited
    • twitter-text/package.json: not the expected form (check: exports)

Code Reviews

Because this PR edits the configuration file, it can be merged once it's reviewed by a DT maintainer.

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 which affect module config files

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

Inactive

This PR has been inactive for 12 days — please try to get reviewers!

{
  "type": "info",
  "now": "-",
  "pr_number": 65147,
  "author": "reosablo",
  "headCommitOid": "4c37be51fe8c00f79a496ca85a3fe7778b6d431f",
  "mergeBaseOid": "88ab488c7d6c4ff0429a24b5822dd8e9c66a8240",
  "lastPushDate": "2023-04-16T06:18:02.000Z",
  "lastActivityDate": "2023-04-19T17:49:15.000Z",
  "hasMergeConflict": false,
  "isFirstContribution": true,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Well-liked by everyone",
  "pkgInfo": [
    {
      "name": "twitter-text",
      "kind": "edit",
      "files": [
        {
          "path": "types/twitter-text/OTHER_FILES.txt",
          "kind": "package-meta",
          "suspect": "not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-other_filestxt)"
        },
        {
          "path": "types/twitter-text/index.d.mts",
          "kind": "package-meta",
          "suspect": "edited"
        },
        {
          "path": "types/twitter-text/package.json",
          "kind": "package-meta",
          "suspect": "not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-packagejson) (check: `exports`)"
        }
      ],
      "owners": [
        "rhysd"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    }
  ],
  "reviews": [],
  "mainBotCommentID": 1510145796,
  "ciResult": "pass"
}

🔔 @rhysd — 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.

I referenced these codes to create this PR:

• types/uuid
• (marked) Add export map and esm entrypoint #57236

ESM imports and CJS requires of twitter-text actually resolve to the same CJS module. The reason for the difference and the unsoundness in the types is that Node uses syntactic analysis to find module.exports properties that can become named exports (and likewise properties on the import * module namespace object), and the lexer doesn’t understand the pattern used by twitter-text. When looking at the types for this module, TypeScript can’t possibly know which properties of the exported object can or can’t be syntactically hoisted, so it assumes that everything can. The types in this package are already correct; this is just a known limitation for TypeScript in all cases where an ESM file imports a CJS file in Node. Sometimes TypeScript will tell you that you can use named imports or a namespace import, but in reality you need to use the default import and access properties on that. It’s an unfortunate situation.

The major downside to the workaround you’ve proposed here is it artificially blocks access to all deep imports to twitter-text. Because the implementation package doesn’t have a package.json "exports", imports like "twitter-text/dist/autoLink.js" are allowed to function. But those imports will be prohibited (not just implicitly any as they are today) with the addition of the package.json "exports". The rule of thumb is that the DT package.json structure needs to perfectly represent the implementation package.json structure.

It might be possible to iterate on the workaround you have here to mitigate some of the unintended side effects such that it would be technically still lying about the implementation package structure, but observably correct. However, because this is a widespread issue, I don’t think it’s worth it to create workarounds for individual packages. I think we’re better off brainstorming how to fix this problem in general in TypeScript.

Re-ping @rhysd:

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

Could someone please give it some attention? Thanks!

I'll close this PR based on the earlier comment.

@andrewbranch
Thank you for the detailed explanation!
I tried a few scenarios myself and now understand what you meant.