From 24ac85c8de46f6218c619cc71c4ebdf992b66f48 Mon Sep 17 00:00:00 2001 From: Tommy Wiebell Date: Wed, 22 May 2019 14:12:18 -0500 Subject: [PATCH 1/3] - Generate optimized URLs using new GraphQL query result - Update tests to reflect functionality change with absolute URLs --- .../src/util/__tests__/makeUrl.spec.js | 14 +--- packages/venia-concept/src/util/makeUrl.js | 83 ++++++++++--------- .../venia-concept/templates/open-document.mst | 2 +- packages/venia-concept/venia-upward.yml | 12 ++- 4 files changed, 60 insertions(+), 51 deletions(-) diff --git a/packages/venia-concept/src/util/__tests__/makeUrl.spec.js b/packages/venia-concept/src/util/__tests__/makeUrl.spec.js index 171bd26f1a..1a55ea00b0 100644 --- a/packages/venia-concept/src/util/__tests__/makeUrl.spec.js +++ b/packages/venia-concept/src/util/__tests__/makeUrl.spec.js @@ -30,18 +30,12 @@ test('prepends media path for product images', () => { expect(makeUrl(relativePath, { type: 'image-product' })).toBe( `${productBase}${relativePath}?${defaultParams}` ); - expect(makeUrl(absoluteUrls[2], { type: 'image-product' })).toBe( - 'https://example.com/media/catalog/product/baz.png?auto=webp&format=pjpg' - ); }); test('prepends media path for relative category images', () => { expect(makeUrl(relativePath, { type: 'image-category' })).toBe( `${categoryBase}${relativePath}?${defaultParams}` ); - expect(makeUrl(absoluteUrls[2], { type: 'image-category' })).toBe( - 'https://example.com/media/catalog/category/baz.png?auto=webp&format=pjpg' - ); }); test("doesn't prepend media path if it's already included", () => { @@ -52,12 +46,12 @@ test("doesn't prepend media path if it's already included", () => { ).toBeTruthy(); }); -test('rewrites absolute url when width is provided', () => { +test('appends opt params to absolute url when media base and width it provided', () => { const width = 100; const raw = absoluteUrls[2]; expect(makeUrl(raw, { type: 'image-product', width })).toBe( - `https://example.com${productBase}/baz.png?auto=webp&format=pjpg&width=100` + `https://example.com/baz.png?auto=webp&format=pjpg&width=100` ); }); @@ -73,7 +67,7 @@ test('removes absolute origin if configured to', () => { jest.resetModules(); const width = 100; const htmlTag = document.querySelector('html'); - htmlTag.setAttribute('data-backend', 'https://cdn.origin:8000/'); + htmlTag.setAttribute('data-media-backend', 'https://cdn.origin:8000/'); htmlTag.setAttribute('data-image-optimizing-origin', 'onboard'); const makeUrlAbs = require('../makeUrl').default; expect( @@ -88,7 +82,7 @@ test('prepends absolute origin if configured to', () => { jest.resetModules(); const width = 100; const htmlTag = document.querySelector('html'); - htmlTag.setAttribute('data-backend', 'https://cdn.origin:9000/'); + htmlTag.setAttribute('data-media-backend', 'https://cdn.origin:9000/'); htmlTag.setAttribute('data-image-optimizing-origin', 'backend'); const makeUrlAbs = require('../makeUrl').default; expect( diff --git a/packages/venia-concept/src/util/makeUrl.js b/packages/venia-concept/src/util/makeUrl.js index b4ac4ed6d1..ebe427d0b7 100644 --- a/packages/venia-concept/src/util/makeUrl.js +++ b/packages/venia-concept/src/util/makeUrl.js @@ -1,34 +1,36 @@ // If the root template supplies the backend URL at runtime, use it directly -const { imageOptimizingOrigin, backend } = document.querySelector( - 'html' -).dataset; -const useBackendForImgs = backend && imageOptimizingOrigin === 'backend'; +const { + imageOptimizingOrigin, + mediaBackend = 'https://backend.test/media/' +} = document.querySelector('html').dataset; +const useBackendForImgs = imageOptimizingOrigin === 'backend'; + +// Tests if a URL begins with `http://` or `https://` or `data:` +const absoluteUrl = /^(data|http|https)?:/i; + +// Simple path joiner that guarantees one and only one slash between segments +const joinUrls = (base, url) => + (base.endsWith('/') ? base.slice(0, -1) : base) + + '/' + + (url.startsWith('/') ? url.slice(1) : url); const mediaBases = new Map() - .set( - 'image-product', - process.env.MAGENTO_BACKEND_MEDIA_PATH_PRODUCT || - '/media/catalog/product' - ) - .set( - 'image-category', - process.env.MAGENTO_BACKEND_MEDIA_PATH_CATEGORY || - '/media/catalog/category' - ); + .set('image-product', 'catalog/product/') + .set('image-category', 'catalog/category/'); /** - * Creates an "optimized" url for a provided absolute or relative url based on + * Creates an "optimized" url for a provided relative url based on * requested media type and width. Any image URLs (whose type begins with * "image-" will also be optimized.) * * If a `type` is provided the `path` will be joined with the associated media * base. - * - `/media/catalog/product/path/to/img.jpg` + * - `catalog/product/path/to/img.jpg` * * If a `width` is provided, "resize parameters" are added to the URL for * middlewares (either onboard or backend) to return using the desired width * and original media url. - * - `/media/catalog/product/path/to/img.jpg?width=500&auto=webp&format=pjpg + * - `catalog/product/path/to/img.jpg?width=500&auto=webp&format=pjpg * * If only `path` is provided it is returned unaltered. * @@ -38,46 +40,49 @@ const mediaBases = new Map() * @param {number} props.width - the desired resize width of the image */ const makeOptimizedUrl = (path, { type, width } = {}) => { - const { href, origin } = window.location; - let urlObject = new URL(path, href); + // Immediate return if there's no image optimization to attempt + if (!type || !type.startsWith('image-')) { + return path; + } + + const { origin } = window.location; + const isAbsolute = absoluteUrl.test(path); + let baseURL = new URL(path, mediaBackend); if (type) { - if (mediaBases.has(type)) { + // If URL is relative and has a supported type, prepend media base onto path + if (!isAbsolute && mediaBases.has(type)) { const mediaBase = mediaBases.get(type); - // prepend media base if it isn't already part of the pathname - if (!urlObject.pathname.includes(mediaBase)) { - urlObject.pathname = mediaBase + urlObject.pathname; + if (!baseURL.pathname.includes(mediaBase)) { + baseURL = new URL(joinUrls(mediaBase, path), mediaBackend); } } - // add image optimization parameters and optionally change origin + + // If type matches image pattern append image optimization parameters if (type.startsWith('image-')) { - if (useBackendForImgs) { - urlObject = new URL( - urlObject.href.slice(urlObject.origin.length), - backend + if (baseURL.href.startsWith(mediaBackend) && !useBackendForImgs) { + baseURL = new URL( + baseURL.href.slice(baseURL.origin.length), + origin ); - } else if (path.startsWith(backend) && !useBackendForImgs) { - // Some API responses include absolute URLs to images. - // The backend won't optimize images, so do not use this - // absolute URL; instead, use a relative URL which has a chance - // of being passed through image optimization. - urlObject = new URL(path.slice(backend.length), origin); } - const params = new URLSearchParams(urlObject.search); + + const params = new URLSearchParams(baseURL.search); params.set('auto', 'webp'); // Use the webp format if available params.set('format', 'pjpg'); // Use progressive JPGs at least if (width) { // resize! params.set('width', width); } - urlObject.search = params.toString(); + baseURL.search = params.toString(); } } - if (urlObject.origin === origin) { - return urlObject.href.slice(origin.length); + if (baseURL.origin === origin) { + return baseURL.href.slice(baseURL.origin.length); } - return urlObject.href; + + return baseURL.href; }; export default makeOptimizedUrl; diff --git a/packages/venia-concept/templates/open-document.mst b/packages/venia-concept/templates/open-document.mst index fc1fe42c80..d39c8f2c80 100644 --- a/packages/venia-concept/templates/open-document.mst +++ b/packages/venia-concept/templates/open-document.mst @@ -1,5 +1,5 @@ - + diff --git a/packages/venia-concept/venia-upward.yml b/packages/venia-concept/venia-upward.yml index e74f02f2dd..b726ff3523 100644 --- a/packages/venia-concept/venia-upward.yml +++ b/packages/venia-concept/venia-upward.yml @@ -31,7 +31,7 @@ response: # handled by the top-level 'proxy' object, which is a ProxyResolver # that passes the request through to the backing Magento server. - matches: request.url.pathname - pattern: '^/(graphql|rest|media)(/|$)' + pattern: '^/(graphql|rest)(/|$)' use: proxy # Requests to create an account are handled by the top-level 'index' # object, which returns a generic app shell @@ -129,6 +129,7 @@ appShell: bundles: assetManifest.bundles urlResolver: urlResolver env: env + mediaBackendURL: mediaBackendURL # The resource object provides information about the page type and which # template to use for a requested resource. @@ -238,6 +239,15 @@ urlResolverResult: inline: urlKey: request.url.pathname +# Extract base media URL from ServiceResolver result +mediaBackendURL: mediaBackendResult.data.storeConfig.secure_base_media_url + +# Retrieve base media URL with ServiceResolver query +mediaBackendResult: + url: magentoGQL + query: + inline: 'query { storeConfig { secure_base_media_url } }' + # The magentoGQL object is a UrlResolver that returns the URL for the # GraphQL endpoint using the MAGENTO_BACKEND_URL environment variable. magentoGQL: From daa853c8f7101c0987f0e28199046bb397151dc3 Mon Sep 17 00:00:00 2001 From: Tommy Wiebell Date: Fri, 24 May 2019 09:28:20 -0500 Subject: [PATCH 2/3] Remove obsolete media path variables --- packages/venia-concept/.env.dist | 7 ------- packages/venia-concept/validate-environment.js | 12 ------------ 2 files changed, 19 deletions(-) diff --git a/packages/venia-concept/.env.dist b/packages/venia-concept/.env.dist index a4e54253b1..913fc29bb8 100644 --- a/packages/venia-concept/.env.dist +++ b/packages/venia-concept/.env.dist @@ -22,13 +22,6 @@ MAGENTO_BACKEND_URL="https://release-dev-231-npzdaky-zddsyhrdimyra.us-4.magentos # you can change this variable. Example: # MAGENTO_WEBSITE_CODE="foo" # -# Change the URL paths where the PWA expects Magento to serve catalog media. -# By default these are `/media/catalog/product` and `/media/catalog/category`, -# but if your Magento instance does something different, they're configurable -# here. -# MAGENTO_BACKEND_MEDIA_PATH_PRODUCT="/media/catalog/product" -# MAGENTO_BACKEND_MEDIA_PATH_CATEGORY="/media/catalog/category" -# # These legacy environment variables from 1.x are no longer necessary: # - `MAGENTO_BACKEND_PUBLIC_PATH`: Since PWA Studio no longer makes _themes_, # it is no longer necessary to follow theme directory structure. diff --git a/packages/venia-concept/validate-environment.js b/packages/venia-concept/validate-environment.js index 4bb5f915b6..5c879990c4 100644 --- a/packages/venia-concept/validate-environment.js +++ b/packages/venia-concept/validate-environment.js @@ -36,18 +36,6 @@ function validateEnvironment(env) { example: 'sw.js', default: 'sw.js' }), - MAGENTO_BACKEND_MEDIA_PATH_PRODUCT: str({ - desc: - 'URL path where the PWA expects Magento to serve product media.', - example: '/media/catalog/product', - default: '/media/catalog/product' - }), - MAGENTO_BACKEND_MEDIA_PATH_CATEGORY: str({ - desc: - 'URL path where the PWA expects Magento to serve category media.', - example: '/media/catalog/category', - default: '/media/catalog/category' - }), MAGENTO_BUILDPACK_PROVIDE_SECURE_HOST: bool({ desc: 'On first run, create a secure, unique hostname and generate a trusted SSL certificate.', From a21e8da5c40e4410b661f99f1ae5aa4ec1f5216a Mon Sep 17 00:00:00 2001 From: Tommy Wiebell Date: Fri, 31 May 2019 11:14:54 -0500 Subject: [PATCH 3/3] Address feedback --- .../src/util/__tests__/makeUrl.spec.js | 2 +- packages/venia-concept/src/util/makeUrl.js | 43 ++++++++----------- 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/packages/venia-concept/src/util/__tests__/makeUrl.spec.js b/packages/venia-concept/src/util/__tests__/makeUrl.spec.js index 1a55ea00b0..ac25dc668a 100644 --- a/packages/venia-concept/src/util/__tests__/makeUrl.spec.js +++ b/packages/venia-concept/src/util/__tests__/makeUrl.spec.js @@ -46,7 +46,7 @@ test("doesn't prepend media path if it's already included", () => { ).toBeTruthy(); }); -test('appends opt params to absolute url when media base and width it provided', () => { +test('appends opt params to absolute url when width is provided', () => { const width = 100; const raw = absoluteUrls[2]; diff --git a/packages/venia-concept/src/util/makeUrl.js b/packages/venia-concept/src/util/makeUrl.js index ebe427d0b7..ffeb9bb167 100644 --- a/packages/venia-concept/src/util/makeUrl.js +++ b/packages/venia-concept/src/util/makeUrl.js @@ -5,7 +5,7 @@ const { } = document.querySelector('html').dataset; const useBackendForImgs = imageOptimizingOrigin === 'backend'; -// Tests if a URL begins with `http://` or `https://` or `data:` +// Tests if a URL begins with `http:` or `https:` or `data:` const absoluteUrl = /^(data|http|https)?:/i; // Simple path joiner that guarantees one and only one slash between segments @@ -49,34 +49,27 @@ const makeOptimizedUrl = (path, { type, width } = {}) => { const isAbsolute = absoluteUrl.test(path); let baseURL = new URL(path, mediaBackend); - if (type) { - // If URL is relative and has a supported type, prepend media base onto path - if (!isAbsolute && mediaBases.has(type)) { - const mediaBase = mediaBases.get(type); - if (!baseURL.pathname.includes(mediaBase)) { - baseURL = new URL(joinUrls(mediaBase, path), mediaBackend); - } + // If URL is relative and has a supported type, prepend media base onto path + if (!isAbsolute && mediaBases.has(type)) { + const mediaBase = mediaBases.get(type); + if (!baseURL.pathname.includes(mediaBase)) { + baseURL = new URL(joinUrls(mediaBase, path), mediaBackend); } + } - // If type matches image pattern append image optimization parameters - if (type.startsWith('image-')) { - if (baseURL.href.startsWith(mediaBackend) && !useBackendForImgs) { - baseURL = new URL( - baseURL.href.slice(baseURL.origin.length), - origin - ); - } + if (baseURL.href.startsWith(mediaBackend) && !useBackendForImgs) { + baseURL = new URL(baseURL.href.slice(baseURL.origin.length), origin); + } - const params = new URLSearchParams(baseURL.search); - params.set('auto', 'webp'); // Use the webp format if available - params.set('format', 'pjpg'); // Use progressive JPGs at least - if (width) { - // resize! - params.set('width', width); - } - baseURL.search = params.toString(); - } + // Append image optimization parameters + const params = new URLSearchParams(baseURL.search); + params.set('auto', 'webp'); // Use the webp format if available + params.set('format', 'pjpg'); // Use progressive JPGs at least + if (width) { + // resize! + params.set('width', width); } + baseURL.search = params.toString(); if (baseURL.origin === origin) { return baseURL.href.slice(baseURL.origin.length);