Skip to content

Commit

Permalink
fix server transformer not respecting host and port overrides (#33425)
Browse files Browse the repository at this point in the history
* fix server transformer not respecting host and port overrides

* Update build-system/server/new-server/transforms/transform.ts

Co-authored-by: Raghu Simha <rsimha@amp.dev>

* Update build-system/server/new-server/transforms/transform.ts

Co-authored-by: Raghu Simha <rsimha@amp.dev>

* fix tests

* add CDNURLToLocalHostRelativeAbsoluteDist and use it in script transform

Co-authored-by: Raghu Simha <rsimha@amp.dev>
  • Loading branch information
erwinmombay and rsimha committed Mar 23, 2021
1 parent 2d01a12 commit 7dac0bb
Show file tree
Hide file tree
Showing 13 changed files with 38 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import posthtml from 'posthtml';
import {isJsonScript, isValidScript, tryGetUrl} from '../utilities/script';
import {CDNURLToLocalDistURL} from '../utilities/cdn';
import {CDNURLToLocalHostRelativeAbsoluteDist} from '../utilities/cdn';
import {OptionSet} from '../utilities/option-set';
import {parse} from 'path';

Expand All @@ -38,7 +38,7 @@ function modifySrc(script: posthtml.Node, options: OptionSet): posthtml.Node {

const url = tryGetUrl(script.attrs.src || '');
const parsedPath = parse(url.pathname);
const src = CDNURLToLocalDistURL(url, [null, null], parsedPath.ext, options.port, options.useMaxNames)
const src = CDNURLToLocalHostRelativeAbsoluteDist(url, [null, null], parsedPath.ext, options.port, options.useMaxNames)
.toString();
script.attrs.src = src;
return script;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<script src="https://cdn.ampproject.org/v0.js" async></script>
<script src="https://cdn.ampproject.org/v0.js?a=b&c=d#hello=world" async></script>
<script src="https://cdn.ampproject.org/v0/amp-bind-0.1.js" async></script>
<script src="https://cdn.ampproject.org/v0/amp-bind-0.2.js" async></script>
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<script src="http://localhost:8000/dist/amp.js" async=""></script>
<script src="http://localhost:8000/dist/v0/amp-bind-0.1.max.js" async=""></script>
<script src="http://localhost:8000/dist/v0/amp-bind-0.2.max.js" async=""></script>
<script src="/dist/amp.js?a=b&c=d#hello=world" async=""></script>
<script src="/dist/v0/amp-bind-0.1.max.js" async=""></script>
<script src="/dist/v0/amp-bind-0.2.max.js" async=""></script>
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<script src="http://localhost:8000/dist/v0.js" async=""></script>
<script src="http://localhost:8000/dist/amp.js" async=""></script>
<script src="http://localhost:8000/dist/amp.js" async=""></script>
<script src="/dist/v0.js" async=""></script>
<script src="/dist/amp.js" async=""></script>
<script src="/dist/amp.js" async=""></script>
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
<script src="http://localhost:8000/dist/v0.js" async=""></script>
<script src="http://localhost:8000/dist/v0.js" async=""></script>
<script src="/dist/v0.js" async=""></script>
<script src="/dist/v0.js" async=""></script>
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<script async="" src="http://localhost:9876/dist/amp.js"></script>
<script async="" src="http://localhost:9876/dist/v0.js" nomodule=""></script>
<script async="" src="http://localhost:9876/dist/v0.mjs" type="module"></script>
<script async="" src="http://localhost:9876/dist/v0/amp-bind-0.1.js"></script>
<script async="" src="http://localhost:9876/dist/dist/amp.js"></script>
<script async="" src="/dist/amp.js"></script>
<script async="" src="/dist/v0.js" nomodule=""></script>
<script async="" src="/dist/v0.mjs" type="module"></script>
<script async="" src="/dist/v0/amp-bind-0.1.js"></script>
<script async="" src="/dist/dist/amp.js"></script>
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<script async="" src="http://localhost:8000/amp.js"></script>
<script async="" src="http://localhost:9876/dist/v0.js" nomodule=""></script>
<script async="" src="http://localhost:9876/dist/v0.mjs" type="module"></script>
<script async="" src="/dist/v0.js" nomodule=""></script>
<script async="" src="/dist/v0.mjs" type="module"></script>
<script async="" src="http://localhost:8000/v0/amp-bind-0.1.js"></script>
<script async="" src="/dist/amp.js"></script>
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<script async="" src="http://localhost:8000/dist/v0.js"></script>
<script async="" src="http://localhost:8000/dist/v0.mjs"></script>
<script async="" src="http://localhost:8000/dist/v0.sxg.js"></script>
<script async="" src="/dist/v0.js"></script>
<script async="" src="/dist/v0.mjs"></script>
<script async="" src="/dist/v0.sxg.js"></script>
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
<script src="https://cdn.ampproject.org/v0.js" async></script>
<script src="https://cdn.ampproject.org/v0.js?a=b&c=d#hello=world" async></script>
<script src="https://cdn.ampproject.org/v0/amp-bind-0.1.js" async></script>
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
<script src="http://localhost:8000/dist/v0.js" async=""></script>
<script src="http://localhost:8000/dist/v0/amp-bind-0.1.js" async=""></script>
<script src="/dist/v0.js?a=b&c=d#hello=world" async=""></script>
<script src="/dist/v0/amp-bind-0.1.js" async=""></script>
2 changes: 1 addition & 1 deletion build-system/server/new-server/transforms/transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import transformCss from './css/css-transform';
const argv = minimist(process.argv.slice(2));
const FOR_TESTING = argv._.includes('integration');
// Use 9876 if running integration tests as this is the KARMA_SERVER_PORT
const PORT = FOR_TESTING ? 9876 : 8000;
const PORT = FOR_TESTING ? 9876 : (argv.port ?? 8000);
const ESM = !!argv.esm;

const defaultTransformConfig = {
Expand Down
11 changes: 11 additions & 0 deletions build-system/server/new-server/transforms/utilities/cdn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,17 @@ export function CDNURLToLocalDistURL(
return url;
}

export function CDNURLToLocalHostRelativeAbsoluteDist(
url: URL,
pathnames: [string | null, string | null] = [null, null],
extension: string = '.js',
port: number = 8000,
useMaxNames = false,
): string {
const newUrl = CDNURLToLocalDistURL(url, pathnames, extension, port, useMaxNames);
return `${newUrl.pathname}${newUrl.search}${newUrl.hash}`;
}

/**
* Convert an existing URL to one from a specific RTV.
*/
Expand Down
11 changes: 2 additions & 9 deletions build-system/server/new-server/transforms/utilities/script.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,6 @@ export function toExtension(url: URL, extension: string): URL {
* This is a temporary measure to allow for a relaxed parsing of our
* fixture files' src urls before they are all fixed accordingly.
*/
export function tryGetUrl(src: string, port: number = 8000): URL {
let url;
try {
url = new URL(src);
} catch (e) {
url = new URL(src, `http://localhost:${port}`);
} finally {
return url as URL;
}
export function tryGetUrl(src: string, host: string = '0.0.0.0', port: number = 8000): URL {
return new URL(src, `http://${host}:${port}`);
}

0 comments on commit 7dac0bb

Please sign in to comment.