-
Notifications
You must be signed in to change notification settings - Fork 5
Conversation
* simpler? asset path calculation * restructred more completely
… rhys/use-hashed-bundle * 'master' of https://github.com/Financial-Times/n-ui: Rhys/deploy hashed assets (#986) tests run in prod env (#985) Make edition switcher URL configurable. (#981)
… rhys/use-hashed-bundle * 'master' of https://github.com/Financial-Times/n-ui: change conditional order for rebuild (#988)
build/app/download-assets.js
Outdated
}) | ||
.then(text => { | ||
// if it's an empty string, something probably went wrong | ||
if (!text.length) { | ||
throw new Error('Fetched empty n-ui stylesheet'); | ||
throw new Error('Fetched empty ${s3Name} from s3'); |
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.
Backticks
build/app/download-assets.js
Outdated
.then(res => { | ||
if (res.ok) { | ||
return res.text(); | ||
} | ||
throw new Error('Failed to fetch n-ui stylesheet'); | ||
throw new Error('Failed to download ${s3Name} from s3'); |
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.
Backticks
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.
🙀
const assetHashes = loadAssetHashesJson(`${directory}/public/asset-hashes.json`); | ||
const nUiAssetHashes = loadAssetHashesJson(`${directory}/public/n-ui-asset-hashes.json`); | ||
const nUiReleaseName = nUiManager.getReleaseName(directory); | ||
const nUiUnhashedAssetsRoot = useLocalAppShell ? '/${appName}/n-ui/' : `//www.ft.com/__assets/n-ui/cached/${nUiReleaseName}/`; |
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.
Backticks?
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.
should probably add some tests for dev mode. Not in this PR though as it's already epic
Also contains a load of (hopefully) simplification of how paths to assets are built.