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

Decide if 'internal' JavaScript files should be exported from the package #3904

Closed
2 tasks done
Tracked by #3478
36degrees opened this issue Jul 5, 2023 · 2 comments
Closed
2 tasks done
Tracked by #3478
Milestone

Comments

@36degrees
Copy link
Contributor

36degrees commented Jul 5, 2023

What

Decide if we want to restrict the files that we export (through the exports field in package.json, affecting which files can be resolved through package resolution) to only those we consider to be public.

Why

The NodeJS docs on the exports field suggest that it can be used to 'clearly define the public interface for [the] package' by only exporting the files that we consider to be public.

At the minute we export everything using "./*", however there are only a number of entrypoints that we consider public (the top-level all and the individual components).

Everything in these files is currently private:

  • common/closest-attribute-value.mjs
  • common/index.mjs
  • common/normalise-dataset.mjs
  • i18n.mjs – we have plans to make it public in the future (Make the I18n class public #2977)

And it's ambiguous whether this file is public or private:

  • common/govuk-frontend-version.mjs

Removing files we consider internal / private from the exports will mean that they cannot be imported by consumers of the package – attempting to do so will result in ERR_PACKAGE_PATH_NOT_EXPORTED error being thrown.

Further details

This follows on from a discussion in #3726 (review).

Who needs to work on this

Developers

Who needs to review this

Developers

Done when

  • We've made a decision about whether to continue including internal modules in our package exports, or to restrict them to just those files we consider public
  • The exports field in package.json reflects that decision
@36degrees
Copy link
Contributor Author

Added to the agenda for next Monday's dev catch up

@36degrees
Copy link
Contributor Author

We discussed this in today's dev catch up.

We decided that for now we will continue to export everything from our package, because:

  • it's less likely that we'll break something, either now or in the future, by e.g. having mismatched exports to what we actually want to export
  • it allows users to access our internal API items if they need to (either for use at risk or to patch something)
  • the review app currently 'cheats' and imports things from the src directory, despite the src directory not being part of the published package – restricting the exports would prevent this and require us to make further changes to the review app

We will instead adopt API Extractor's concept of release tags and mark internal classes and functions as @internal.

We can revisit this in the future if we find that we need to be stricter about what users can import from the published package.

@36degrees 36degrees moved this from Sprint Backlog 🏃🏼‍♀️ to Done 🏁 in GOV.UK Design System cycle board Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

1 participant