-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
misc(publish): include the report bundle in npm package #13349
Conversation
What's wrong with a client using We could even provide an |
looks good. tried it out with a local web.dev (thank you esm, too. ;) |
package.json
Outdated
"exports": { | ||
".": "./lighthouse-core/index.js", | ||
"./renderer": "./dist/report/bundle.esm.js" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When exports is defined, it means only files that match one of these rules may be imported from the package. See the output from yarn build-devtools
for how this breaks pubads:
Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './lighthouse-core/lib/i18n/i18n' is not defined by "exports" in /Users/runner/work/lighthouse/lighthouse/lighthouse/node_modules/lighthouse/package.json
Adding this line should fix it:
"./lighthouse-core/*": "./lighthouse-core/*"
@connorjclark can we merge as is? i certainly don't want to block v9 release with a "lets lock down our node exports" exercise. We've already found that it's not trivial with just one of our many downstream customers. :( |
to support folks like web.dev using our report renderer we need to include the new renderer API in our published assets