From 60cb06c1ba32746f5ddeb0fdebbd4af0dc3b5293 Mon Sep 17 00:00:00 2001 From: Arunoda Susiripala Date: Sat, 3 Feb 2018 01:39:24 +0530 Subject: [PATCH] Improved next/asset support (#3664) * Allow next/asset to work properly with dynamic assetPrefix Now we use webpack's publicPath via client side. * Add test cases for dynamic assetPrefix and next/asset. --- client/index.js | 5 +- lib/asset.js | 5 ++ server/build/webpack.js | 1 - .../basic/pages/using-asset/asset.js | 10 ++++ .../basic/pages/using-asset/index.js | 9 +++ test/integration/basic/test/asset.js | 57 +++++++++++++++++++ test/integration/basic/test/index.test.js | 2 + test/integration/custom-server/pages/asset.js | 7 +++ test/integration/custom-server/pages/index.js | 8 ++- test/integration/custom-server/server.js | 2 +- .../custom-server/test/index.test.js | 46 ++++++++++++--- 11 files changed, 140 insertions(+), 12 deletions(-) create mode 100644 test/integration/basic/pages/using-asset/asset.js create mode 100644 test/integration/basic/pages/using-asset/index.js create mode 100644 test/integration/basic/test/asset.js create mode 100644 test/integration/custom-server/pages/asset.js diff --git a/client/index.js b/client/index.js index 78d953e5..f14cb395 100644 --- a/client/index.js +++ b/client/index.js @@ -30,7 +30,10 @@ const { location } = window -// With this, static assets will work across zones +// With dynamic assetPrefix it's no longer possible to set assetPrefix at the build time +// So, this is how we do it in the client side at runtime +__webpack_public_path__ = `${assetPrefix}/_next/webpack/` //eslint-disable-line +// Initialize next/asset with the assetPrefix asset.setAssetPrefix(assetPrefix) const asPath = getURL() diff --git a/lib/asset.js b/lib/asset.js index f9dfe50c..65b7e618 100644 --- a/lib/asset.js +++ b/lib/asset.js @@ -1,6 +1,11 @@ let assetPrefix export default function asset (path) { + // If the URL starts with http, we assume it's an + if (/^https?:\/\//.test(path)) { + return path + } + const pathWithoutSlash = path.replace(/^\//, '') return `${assetPrefix}/static/${pathWithoutSlash}` } diff --git a/server/build/webpack.js b/server/build/webpack.js index a6d5fdce..38241627 100644 --- a/server/build/webpack.js +++ b/server/build/webpack.js @@ -126,7 +126,6 @@ export default async function getBaseWebpackConfig (dir, {dev = false, isServer path: path.join(dir, config.distDir, isServer ? 'dist' : ''), // server compilation goes to `.next/dist` filename: '[name]', libraryTarget: 'commonjs2', - publicPath: `${config.assetPrefix}/_next/webpack/`, // This saves chunks with the name given via require.ensure() chunkFilename: '[name]-[chunkhash].js', strictModuleExceptionHandling: true, diff --git a/test/integration/basic/pages/using-asset/asset.js b/test/integration/basic/pages/using-asset/asset.js new file mode 100644 index 00000000..1dd2976c --- /dev/null +++ b/test/integration/basic/pages/using-asset/asset.js @@ -0,0 +1,10 @@ +import asset from 'next/asset' + +export default () => ( +
+ + + + +
+) diff --git a/test/integration/basic/pages/using-asset/index.js b/test/integration/basic/pages/using-asset/index.js new file mode 100644 index 00000000..9e1e3b56 --- /dev/null +++ b/test/integration/basic/pages/using-asset/index.js @@ -0,0 +1,9 @@ +import Link from 'next/link' + +export default () => ( +
+ + Asset + +
+) diff --git a/test/integration/basic/test/asset.js b/test/integration/basic/test/asset.js new file mode 100644 index 00000000..886ec28d --- /dev/null +++ b/test/integration/basic/test/asset.js @@ -0,0 +1,57 @@ +/* global describe, it, expect */ +import cheerio from 'cheerio' +import { + renderViaHTTP +} from 'next-test-utils' +import webdriver from 'next-webdriver' + +export default (context) => { + async function get$ (path) { + const html = await renderViaHTTP(context.appPort, path) + return cheerio.load(html) + } + + describe('With next/asset', () => { + describe('with SSR', () => { + it('should handle beginning slash properly', async () => { + const $ = await get$('/using-asset/asset') + expect($('#img1').attr('src')).toBe('/static/the-image') + expect($('#img2').attr('src')).toBe('/static/the-image') + }) + + it('should handle http(s) properly', async () => { + const $ = await get$('/using-asset/asset') + expect($('#img3').attr('src')).toBe('http://the-image.com/the-image') + expect($('#img4').attr('src')).toBe('https://the-image.com/the-image') + }) + }) + + describe('with client navigation', () => { + it('should handle beginning slash properly', async () => { + const browser = await webdriver(context.appPort, '/using-asset') + await browser + .elementByCss('#go-asset').click() + .waitForElementByCss('#asset-page') + + expect(await browser.elementByCss('#img1').getAttribute('src')) + .toBe(`http://localhost:${context.appPort}/static/the-image`) + expect(await browser.elementByCss('#img2').getAttribute('src')) + .toBe(`http://localhost:${context.appPort}/static/the-image`) + browser.close() + }) + + it('should handle http(s) properly', async () => { + const browser = await webdriver(context.appPort, '/using-asset') + await browser + .elementByCss('#go-asset').click() + .waitForElementByCss('#asset-page') + + expect(await browser.elementByCss('#img3').getAttribute('src')) + .toBe('http://the-image.com/the-image') + expect(await browser.elementByCss('#img4').getAttribute('src')) + .toBe('https://the-image.com/the-image') + browser.close() + }) + }) + }) +} diff --git a/test/integration/basic/test/index.test.js b/test/integration/basic/test/index.test.js index f3e71bb7..14ebe3d8 100644 --- a/test/integration/basic/test/index.test.js +++ b/test/integration/basic/test/index.test.js @@ -14,6 +14,7 @@ import rendering from './rendering' import clientNavigation from './client-navigation' import hmr from './hmr' import dynamic from './dynamic' +import asset from './asset' const context = {} jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 5 @@ -60,4 +61,5 @@ describe('Basic Features', () => { clientNavigation(context, (p, q) => renderViaHTTP(context.appPort, p, q)) dynamic(context, (p, q) => renderViaHTTP(context.appPort, p, q)) hmr(context, (p, q) => renderViaHTTP(context.appPort, p, q)) + asset(context) }) diff --git a/test/integration/custom-server/pages/asset.js b/test/integration/custom-server/pages/asset.js new file mode 100644 index 00000000..92b695bb --- /dev/null +++ b/test/integration/custom-server/pages/asset.js @@ -0,0 +1,7 @@ +import asset from 'next/asset' + +export default () => ( +
+ +
+) diff --git a/test/integration/custom-server/pages/index.js b/test/integration/custom-server/pages/index.js index 13e1c831..0e5b18af 100644 --- a/test/integration/custom-server/pages/index.js +++ b/test/integration/custom-server/pages/index.js @@ -1,3 +1,9 @@ +import Link from 'next/link' + export default () => ( -
My Homepage
+
+ + Asset + +
) diff --git a/test/integration/custom-server/server.js b/test/integration/custom-server/server.js index 3c58563c..8547f14a 100644 --- a/test/integration/custom-server/server.js +++ b/test/integration/custom-server/server.js @@ -11,7 +11,7 @@ const handleNextRequests = app.getRequestHandler() app.prepare().then(() => { const server = micro((req, res) => { if (/setAssetPrefix/.test(req.url)) { - app.setAssetPrefix('https://cdn.com/myapp') + app.setAssetPrefix(`http://127.0.0.1:${port}`) } else { // This is to support multi-zones support in localhost // and may be in staging deployments diff --git a/test/integration/custom-server/test/index.test.js b/test/integration/custom-server/test/index.test.js index dcab619f..e476d766 100644 --- a/test/integration/custom-server/test/index.test.js +++ b/test/integration/custom-server/test/index.test.js @@ -3,11 +3,13 @@ import { join } from 'path' import getPort from 'get-port' import clone from 'clone' +import cheerio from 'cheerio' import { initNextServerScript, killApp, renderViaHTTP } from 'next-test-utils' +import webdriver from 'next-webdriver' const appDir = join(__dirname, '../') let appPort @@ -29,23 +31,51 @@ describe('Custom Server', () => { describe('with dynamic assetPrefix', () => { it('should set the assetPrefix dynamically', async () => { - const normalUsage = await renderViaHTTP(appPort, '/') - expect(normalUsage).not.toMatch(/cdn\.com\/myapp/) + const normalUsage = await renderViaHTTP(appPort, '/asset') + expect(normalUsage).not.toMatch(/127\.0\.0\.1/) - const dynamicUsage = await renderViaHTTP(appPort, '/?setAssetPrefix=1') - expect(dynamicUsage).toMatch(/cdn\.com\/myapp/) + const dynamicUsage = await renderViaHTTP(appPort, '/asset?setAssetPrefix=1') + expect(dynamicUsage).toMatch(/127\.0\.0\.1/) }) it('should set the assetPrefix to a given request', async () => { for (let lc = 0; lc < 1000; lc++) { const [normalUsage, dynamicUsage] = await Promise.all([ - await renderViaHTTP(appPort, '/'), - await renderViaHTTP(appPort, '/?setAssetPrefix=1') + await renderViaHTTP(appPort, '/asset'), + await renderViaHTTP(appPort, '/asset?setAssetPrefix=1') ]) - expect(normalUsage).not.toMatch(/cdn\.com\/myapp/) - expect(dynamicUsage).toMatch(/cdn\.com\/myapp/) + expect(normalUsage).not.toMatch(/127\.0\.0\.1/) + expect(dynamicUsage).toMatch(/127\.0\.0\.1/) } }) + + it('should support next/asset in server side', async () => { + const $normal = cheerio.load(await renderViaHTTP(appPort, '/asset')) + expect($normal('img').attr('src')).toBe('/static/myimage.png') + + const $dynamic = cheerio.load(await renderViaHTTP(appPort, '/asset?setAssetPrefix=1')) + expect($dynamic('img').attr('src')).toBe(`http://127.0.0.1:${context.appPort}/static/myimage.png`) + }) + + it('should support next/asset in client side', async() => { + const browser = await webdriver(context.appPort, '/') + await browser + .elementByCss('#go-asset').click() + .waitForElementByCss('#asset-page') + + expect(await browser.elementByCss('img').getAttribute('src')) + .toBe(`http://localhost:${context.appPort}/static/myimage.png`) + browser.close() + + const browser2 = await webdriver(context.appPort, '/?setAssetPrefix=1') + await browser2 + .elementByCss('#go-asset').click() + .waitForElementByCss('#asset-page') + + expect(await browser2.elementByCss('img').getAttribute('src')) + .toBe(`http://127.0.0.1:${context.appPort}/static/myimage.png`) + browser2.close() + }) }) })