-
-
Notifications
You must be signed in to change notification settings - Fork 375
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/prerender urls esm #1667
Fix/prerender urls esm #1667
Changes from all commits
03a2dec
a506f11
2e12616
0d21e8f
89a91dd
ccafe17
b9613c1
91d7501
4412b7d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'preact-cli': patch | ||
--- | ||
|
||
Allows users to author prerender-urls.js as ESM once again |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,8 +5,7 @@ const HtmlWebpackExcludeAssetsPlugin = require('html-webpack-exclude-assets-plug | |
const HtmlWebpackPlugin = require('html-webpack-plugin'); | ||
const prerender = require('./prerender'); | ||
const createLoadManifest = require('./create-load-manifest'); | ||
const { warn } = require('../../util'); | ||
const { info } = require('../../util'); | ||
const { esmImport, tryResolveConfig, warn } = require('../../util'); | ||
|
||
const PREACT_FALLBACK_URL = '/200.html'; | ||
|
||
|
@@ -107,21 +106,27 @@ module.exports = async function (config) { | |
let pages = [{ url: '/' }]; | ||
|
||
if (config.prerenderUrls) { | ||
if (existsSync(resolve(cwd, config.prerenderUrls))) { | ||
const prerenderUrls = tryResolveConfig( | ||
cwd, | ||
config.prerenderUrls, | ||
config.prerenderUrls === 'prerender-urls.json', | ||
config.verbose | ||
); | ||
|
||
if (prerenderUrls) { | ||
try { | ||
let result = require(resolve(cwd, config.prerenderUrls)); | ||
let result = esmImport(prerenderUrls); | ||
|
||
if (typeof result.default !== 'undefined') { | ||
result = result.default(); | ||
result = result.default; | ||
} | ||
if (typeof result === 'function') { | ||
info(`Fetching URLs from ${config.prerenderUrls}`); | ||
result = await result(); | ||
info(`Fetched URLs from ${config.prerenderUrls}`); | ||
} | ||
if (typeof result === 'string') { | ||
result = JSON.parse(result); | ||
} | ||
if (result instanceof Array) { | ||
if (Array.isArray(result)) { | ||
Comment on lines
-124
to
+129
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure why this stopped working, but probably shouldn't be using |
||
pages = result; | ||
} | ||
} catch (error) { | ||
|
@@ -131,17 +136,6 @@ module.exports = async function (config) { | |
}` | ||
); | ||
} | ||
// don't warn if the default file doesn't exist | ||
} else if ( | ||
config.prerenderUrls !== 'prerender-urls.json' || | ||
config.verbose | ||
) { | ||
warn( | ||
`prerenderUrls file (${resolve( | ||
cwd, | ||
config.prerenderUrls | ||
)}) doesn't exist, using default!` | ||
); | ||
} | ||
} | ||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,9 +34,9 @@ | |
"homepage": "https://github.com/preactjs/preact-cli", | ||
"devDependencies": { | ||
"@types/express": "^4.17.13", | ||
"@types/jest": "^27.4.0", | ||
"@types/jest": "^24.9.1", | ||
"html-looks-like": "^1.0.2", | ||
"jest": "^27.0.1", | ||
"jest": "^24.9.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Chased down the weird config file hack I introduced in #1623, turns out to be an issue in 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. |
||
"less": "^4.1.1", | ||
"less-loader": "^7.3.0", | ||
"ncp": "^2.0.0", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
const { join } = require('path'); | ||
const { access } = require('fs').promises; | ||
const { build } = require('./lib/cli'); | ||
const { subject } = require('./lib/output'); | ||
|
||
const formats = ['cjs', 'esm']; | ||
|
||
const prerenderUrlFiles = [ | ||
'array.js', | ||
'stringified-array.js', | ||
'function-returning-array.js', | ||
'function-returning-stringified-array.js', | ||
]; | ||
|
||
const preactConfigFiles = ['function.js', 'object.js']; | ||
|
||
describe('config files', () => { | ||
describe('prerender-urls', () => { | ||
it(`should load the 'prerender-urls.json' file`, async () => { | ||
let dir = await subject('multiple-config-files'); | ||
|
||
await build(dir); | ||
|
||
expect(await access(join(dir, 'build/index.html'))).toBeUndefined(); | ||
expect( | ||
await access(join(dir, 'build/custom/index.html')) | ||
).toBeUndefined(); | ||
}); | ||
|
||
formats.forEach(moduleFormat => { | ||
prerenderUrlFiles.forEach(dataFormat => { | ||
it(`should load the '${dataFormat}' file in ${moduleFormat}`, async () => { | ||
let dir = await subject('multiple-config-files'); | ||
|
||
await build(dir, { | ||
prerenderUrls: `prerenderUrls/${moduleFormat}/${dataFormat}`, | ||
}); | ||
|
||
expect(await access(join(dir, 'build/index.html'))).toBeUndefined(); | ||
expect( | ||
await access(join(dir, 'build/custom/index.html')) | ||
).toBeUndefined(); | ||
}); | ||
}); | ||
}); | ||
}); | ||
|
||
describe('preact.config', () => { | ||
formats.forEach(moduleFormat => { | ||
preactConfigFiles.forEach(dataFormat => { | ||
it(`should load the '${dataFormat}' file in ${moduleFormat}`, async () => { | ||
let dir = await subject('multiple-config-files'); | ||
|
||
await build(dir, { | ||
config: `preactConfig/${moduleFormat}/${dataFormat}`, | ||
}); | ||
|
||
expect(await access(join(dir, 'build/bundle.js'))).toBeUndefined(); | ||
}); | ||
}); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import { h, Component } from 'preact'; | ||
import { Router } from 'preact-router'; | ||
import Home from './routes/home'; | ||
|
||
export default class App extends Component { | ||
handleRoute = e => { | ||
this.currentUrl = e.url; | ||
}; | ||
|
||
render(props) { | ||
return ( | ||
<div id="app"> | ||
<Router url={props.url} onChange={this.handleRoute} {...props}> | ||
<Home path="/" /> | ||
</Router> | ||
</div> | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
{ | ||
"private": true, | ||
"name": "preact-config" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
module.exports = function (config) { | ||
config.output.filename = '[name].js'; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
module.exports = { | ||
webpack(config) { | ||
config.output.filename = '[name].js'; | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export default function (config) { | ||
config.output.filename = '[name].js'; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
export default { | ||
webpack(config) { | ||
config.output.filename = '[name].js'; | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
[ | ||
{ | ||
"url": "/" | ||
}, | ||
{ | ||
"url": "/custom" | ||
} | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
module.exports = [ | ||
{ | ||
url: '/', | ||
}, | ||
{ | ||
url: '/custom', | ||
}, | ||
]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
module.exports = () => [ | ||
{ | ||
url: '/', | ||
}, | ||
{ | ||
url: '/custom', | ||
}, | ||
]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
module.exports = () => `[ | ||
{ | ||
"url": "/" | ||
}, | ||
{ | ||
"url": "/custom" | ||
} | ||
]`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
module.exports = `[ | ||
{ | ||
"url": "/" | ||
}, | ||
{ | ||
"url": "/custom" | ||
} | ||
]`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
export default [ | ||
{ | ||
url: '/', | ||
}, | ||
{ | ||
url: '/custom', | ||
}, | ||
]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
export default () => [ | ||
{ | ||
url: '/', | ||
}, | ||
{ | ||
url: '/custom', | ||
}, | ||
]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
export default () => `[ | ||
{ | ||
"url": "/" | ||
}, | ||
{ | ||
"url": "/custom" | ||
} | ||
]`; |
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.
I don't think these info messages really provided any useful information.