Fix/prerender urls esm #1667
Fix/prerender urls esm #1667
Conversation
🦋 Changeset detectedLatest commit: 4412b7d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
One thing that might also be worth bringing up: I noticed that if a user provides a prerender-urls file, and it cannot be found or is malformed, we give a warning and move on with the default values. If the same happens for the preact.config, we throw an error and exit immediately.
Do we want to change this behavior so they match? I can't say I really have a preference for which way we go.
| "jest": "^27.0.1", | ||
| "jest": "^24.9.0", |
There was a problem hiding this comment.
Chased down the weird config file hack I introduced in #1623, turns out to be an issue in esm (the package, not the format) in Jest > v25.
Issue filed at jestjs/jest#12493, but I needed to use ESM within Jest and that hack only worked for CJS config files in our test suite. Downgrading shouldn't be problematic, so I went this route.
| if (result instanceof Array) { | ||
| if (Array.isArray(result)) { |
There was a problem hiding this comment.
Not sure why this stopped working, but probably shouldn't be using instanceof anyhow
| info(`Fetching URLs from ${config.prerenderUrls}`); | ||
| result = await result(); | ||
| info(`Fetched URLs from ${config.prerenderUrls}`); |
There was a problem hiding this comment.
I don't think these info messages really provided any useful information.
rschristian
force-pushed
the
fix/prerender-urls-esm
branch
from
84e5ed6
to
8732592
Compare
rschristian
force-pushed
the
fix/prerender-urls-esm
branch
from
96a14f7
to
89a91dd
Compare
|
I was looking to add similar file handling for the babelConfig in v4 and the difference in preact config vs prerender urls was bothering me so I fixed it up here by extracting out a reusable handler for such situations. No longer will a missing |
rschristian
force-pushed
the
fix/prerender-urls-esm
branch
from
4245c8d
to
91d7501
Compare
What kind of change does this PR introduce?
Bugfix
Did you add tests for your changes?
Yes
Summary
Closes #1662
As the linked issue mentions, we
require()the user'sprerender-urls.jsfile which disallowsimport/exportkeywords. I'm guessing this used to work due to ESM not really being solidified in Node? Not entirely sure.Fixed this by mirroring the process we already use for the user's
preact.config.jsfiles.Does this PR introduce a breaking change?
No