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

bug: packageJson.exports.types should be always be ordered first #294

Open
jlarmstrongiv opened this issue Apr 9, 2023 · 10 comments · May be fixed by #349
Open

bug: packageJson.exports.types should be always be ordered first #294

jlarmstrongiv opened this issue Apr 9, 2023 · 10 comments · May be fixed by #349

Comments

@jlarmstrongiv
Copy link

Failing Example:

{
  "name": "rcva",
  "version": "1.0.6",
  "private": false,
  "description": "React Class Variance Authority 🧬",
  "keywords": [
    "React Class Variance Authority",
    "react-class-variance-authority",
    "classes",
    "classname",
    "classnames",
    "css",
    "rcva",
    "stitches",
    "vanilla-extract",
    "variants",
    "windstitch"
  ],
  "homepage": "https://github.com/jlarmstrongiv/rcva/blob/main/packages/core/README.md",
  "bugs": "https://github.com/jlarmstrongiv/rcva/issues",
  "repository": "https://github.com/jlarmstrongiv/rcva.git",
  "license": "MIT",
  "author": "John L. Armstrong IV",
  "sideEffects": false,
  "type": "module",
  "exports": {
    ".": {
      "require": "./dist/index.cjs",
      "import": "./dist/index.js",
      "types": "./dist/index.d.ts"
    },
    "./package.json": "./package.json"
  },
  "main": "dist/index.cjs",
  "module": "dist/index.js",
  "source": "src/index.tsx",
  "types": "dist/index.d.ts",
  "files": [
    "dist"
  ],
  "scripts": {
    "build": "tsup-node"
  },
  "devDependencies": {
    "@types/react": "^18.0.31",
    "@types/react-dom": "^18.0.11",
    "rimraf": "^4.4.1",
    "tsup": "^6.7.0",
    "typescript": "^5.0.0"
  },
  "peerDependencies": {
    "@leafygreen-ui/polymorphic": "^1.3.0",
    "class-variance-authority": ">= 0.5.1 < 1",
    "react": "^18.0.0",
    "react-dom": "^18.0.0",
    "tailwind-merge": "^1.11.0"
  }
}

More specifically, Should be the first in the object as required by TypeScript. according to publint.dev:

{
  "exports": {
    ".": {
      "require": "./dist/index.cjs",
      "import": "./dist/index.js",
      "types": "./dist/index.d.ts"
    },
    "./package.json": "./package.json"
  }
}

EXPORTS_TYPES_SHOULD_BE_FIRST

Ensure types condition to be the first. The TypeScript docs recommends so, but it's also because the exports field is order-based.

For example, a scenario where both the types and import condition could be active, types should be first so that it matches and returns a .d.ts file, rather than a .js file from the import condition.

@keithamus
Copy link
Owner

PRs welcome!

@jlarmstrongiv
Copy link
Author

@keithamus would the PR include these changes?

  • adding documentation to defaultRules
  • editing the exports key to something like { key: 'exports', over: sortObjectBy(['types']) }
  • patch version bump

Let me know if I missed anything in the outline 😄 if not, it should be a straightforward fix

@keithamus
Copy link
Owner

That’s correct. We use semantic versioning so prefixing your commit fix: will automatically apply and release the patch version.

@fisker
Copy link
Collaborator

fisker commented Apr 9, 2023

If we are doing this, should we also enforce the default to be the last one?

@mrmckeb
Copy link

mrmckeb commented Oct 10, 2024

@keithamus are you still working on this? If not, I might pick it up.

@keithamus
Copy link
Owner

I am happy to accept PRs. I no longer contribute code to this repo as I prefer to let the community do so.

@mrmckeb
Copy link

mrmckeb commented Oct 10, 2024

Apologies, I meant to tag @jlarmstrongiv! I assume that this is not being worked on though?

@GeorgeTaveras1231
Copy link
Contributor

I'll just add to this thread that types should not always be first. The order should be left up to the user of the library and by default, this package should avoid sorting imports and exports unless specified.

The reason types should not always go first is because package developers are free to define any arbitrary set of conditions. A developer may define a custom condition that takes precedence over the types one. For example, define a condition for using a different types file

{
  "exports": {
      "some-arbitrary-condition": {
        "types": "./different-types-file.d.ts"
      },
      "types": "./fallback-types-file.d.ts"
  }
}

@GeorgeTaveras1231
Copy link
Contributor

@mrmckeb @jlarmstrongiv I opened a PR (#336) addressing this - Just thought I'd let you know, I didn't see your comments about potentially working on this before I opened the PR.

In my first comment I suggested not sorting unless specified - my PR takes a more opinionated position on sorting known conditions while leaving unknown conditions in place (mostly).

@mrmckeb
Copy link

mrmckeb commented Mar 2, 2025

Absolutely fine, this had fallen into my backlog - thank you, @GeorgeTaveras1231!

@fisker fisker linked a pull request Mar 2, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants