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 "exports" field in package.json #13359

Closed
connorjclark opened this issue Nov 11, 2021 · 4 comments
Closed

Use "exports" field in package.json #13359

connorjclark opened this issue Nov 11, 2021 · 4 comments
Assignees
Labels

Comments

@connorjclark
Copy link
Collaborator

connorjclark commented Nov 11, 2021

We first tried to introduce this via #13349 for the v9 release. The idea was to define the exports in a non-breaking way, such that all modules under ./lighthouse-core/* / ./lighthouse-cli/* / ./types/* would be exposed + a new export for the renderer api at ./renderer. However, we quickly found that clients (specifically pubads, but if you search GitHb there are tons) tend to import modules with and without a .js extension.

Defining exports like this seemed like it would support optional extension in the import. The first entry that matches the key is used, and the * matches as much as the pattern as possible (includes slashes).

"./lighthouse-core/*.js": "./lighthouse-core/*.js",
"./lighthouse-core/*": "./lighthouse-core/*.js",

However, yarn build-devtools (exercising the pubads code) still failed to resolve all its imports. As this seemed like a huge blocker to adoption for this feature, I began to write a bug report, but first I found these issues in the nodejs project:

nodejs/node#39635

Turns out, "pattern trailers" was an overlooked feature. It was recently added AND backported to 14.

nodejs/Release#690 (comment)

It should land in the next releases across 14+, but IDK when that is. (Ex: 14.18.2 will have the fix, but the latest is 14.18.1. ditto for 17.1.1 see #13359 (comment)). But, it won't be in time for v9.


I can't test it yet, but I believe this will work:

"exports": {
  ".": "./lighthouse-core/index.js",
  "./renderer": "./dist/report/bundle.esm.js",
  "./lighthouse-core/*": "./lighthouse-core/*.js",
  "./lighthouse-core/*.js": "./lighthouse-core/*.js",
  "./lighthouse-cli/*": "./lighthouse-cli/*.js",
  "./lighthouse-cli/*.js": "./lighthouse-cli/*.js",
  "./types/*": "./types/*"
},

(note the order is swapped from the original intuition. not sure why, I'm just copying from the example in the PR)

@connorjclark
Copy link
Collaborator Author

connorjclark commented Nov 11, 2021

The exports field will also allow us to do things like rename folders #13302 without breaking node users.

@connorjclark
Copy link
Collaborator Author

Support for pattern trailers was added in Node 14.19.0 and 16.9.0. If we wanted to move forward with this, we'd have to upgrade our engines property to:

"engines": ">=14.19.0 || >=16.9.0"

The latest 14.x is 14.19.1 and the latest 16.x is 16.14.2. It seems reasonable to bump our required versions to these, but... Node 15 didn't receive a backport because it isn't an LTS (does that mean we don't support it?). And IDK about 17 because it isn't listed in the changelog but various references in the issue say 17.0.0 should have it...

@connorjclark
Copy link
Collaborator Author

yea we don't officially support non LTS (15)

@connorjclark
Copy link
Collaborator Author

We decided not to do this, and just have the changes from renaming folders be fully breaking. It only affects Node users and the fixes are all trivial.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants