-
Notifications
You must be signed in to change notification settings - Fork 333
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
Fix shared task styles.compile()
when loadPaths don’t exist yet
#4516
Conversation
Internally we use `require.resolve()` to find GOV.UK Frontend but Node.js throws “Cannot find module” when the package hasn’t been built yet We can use `packageTypeToPath()` instead
📋 StatsFile sizes
Modules
View stats and visualisations on the review app Action run for 66080dc |
Prettier didn’t make the best choices here
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.
Good spot, neat we can work around not having built yet with packageTypetoPath
⛵
function packageTypeToPath(packageName, options = {}) { | ||
const { modulePath, moduleRoot, type = 'commonjs' } = options | ||
|
||
// Assume package.json is always resolvable |
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.
question Can a package prevent access to its package.json
? If not, can we drop the "Assume", please, to avoid confusion when we read back this code in the future.
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.
Yeah sadly it can, so this is the correct comment 😔
With package exports you can quite easily lock yourself out from package.json access
So you'll often see it as a separate entry point:
https://github.com/isaacs/tshy?tab=readme-ov-file#exports
This PR fixes a bug where a newly cloned repo can't run:
I've tweaked a bit of awful Prettier formatting and added some better comments too
Problem
For our Sass
loadPaths
we userequire.resolve()
viapackageResolveToPath()
to locategovuk-frontend
But Node.js throws “Cannot find module” since
npm run build:package
hasn't been run yetSolution
We can use the
packageResolveToPath()
packageTypeToPath()
helper insteadThis helper was added when Node.js v17 couldn't use
require.resolve()
in #3755