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

Use package.json exports to "hide" the dist folder for packages and control our exported surface-area #6050

Closed
JoshuaKGoldberg opened this issue Nov 20, 2022 Discussed in #6015 · 0 comments
Labels
accepting prs Go ahead, send a pull request that resolves this issue breaking change This change will require a new major version to be released enhancement New feature or request
Milestone

Comments

@JoshuaKGoldberg
Copy link
Member

Discussed in #6015

Originally posted by bradzacher September 1, 2022
Currently we don't use package.json exports, so anyone can import any file in any package by directly referencing a path within the dist folder - i.e.

import * as TSESLint from '@typescript-eslint/utils/dist/ts-eslint';

This is obviously a sub-par experience for consumers due to the deep path, and is a sub-par experience for us because any file technically forms part of our "external API".

In ESLint v8 they began using exports to lock down their package, and I think we should follow their lead to some degree.

I propose that we follow one of the following options:

1) export all of dist, without the dist

{
  "exports": {
    "./*": "./dist/*"
  },
  "typesVersions": {
    "*": {
      "*": ["dist/*"]
    }
  }
}

This is the least restrictive thing we could do - it would simply mean consumers needn't include dist/ in their import path - which is nice as it hides an implementation detail.

2) export all, but ban specific things

{
  "exports": {
    "./*": "./dist/*"
    "./some-internal/file": null
  },
  "typesVersions": {
    "*": {
      "*": ["dist/*"]
    }
  }
}

This is slightly more restrictive, but still very permissive.
This would allow us to build APIs that are intended to be internal only and hide them, whilst still generally allowing all imports without the dist/.

3) export specific, allowed parts of our API

{
  "exports": {
    "./": "./dist/index",
    "./ts-eslint": "./dist/ts-eslint/index",
    "./ast-utils": "./dist/ast-utils/index",
    // ...
  },
  "typesVersions": {
    "*": {
      "./": "./dist/index",
      "./ts-eslint": "./dist/ts-eslint/index.d.ts",
      "./ast-utils": "./dist/ast-utils/index.d.ts",
      // ...
    }
  }
}

This is the most restrictive thing we could do as we would ban deep imports except those that we choose. It would also allow us to remove the dist/ from imports.

Conclusion

We'd like to go with option 3. Marking as a breaking change and accepting PRs! 🚀

@JoshuaKGoldberg JoshuaKGoldberg added enhancement New feature or request breaking change This change will require a new major version to be released accepting prs Go ahead, send a pull request that resolves this issue labels Nov 20, 2022
@JoshuaKGoldberg JoshuaKGoldberg added this to the 6.0.0 milestone Nov 20, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue breaking change This change will require a new major version to be released enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants