Skip to content

Commit

Permalink
Fix problem with sourcemaps and their sources not being moved to CDN
Browse files Browse the repository at this point in the history
  • Loading branch information
Munter committed Oct 18, 2019
1 parent ad30b22 commit 81b5bf4
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 26 deletions.
24 changes: 23 additions & 1 deletion lib/hashfiles.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,25 @@
const { ensureTrailingSlash } = require('urltools');

function sourceMapCircleFilter(relation) {
if (relation.type !== 'SourceMapFile') {
return true;
}

const fromAsset = relation.from;
const toAsset = relation.to;

const sourceMapRelations = [
'JavaScriptSourceMappingUrl',
'JavaScriptSourceUrl'
];

const hasSourceMapFileCircle = toAsset.outgoingRelations.find(
rel => sourceMapRelations.includes(rel.type) && rel.to === fromAsset
);

return !hasSourceMapFileCircle;
}

/**
* @param {AssetGraph} assetGraph An AssetGraph instance with all relevant assets already populated
* @param {object} [options]
Expand Down Expand Up @@ -102,6 +122,8 @@ module.exports = async function hashfiles(assetGraph, options = {}) {
isInline: false
}
})
// Don't block on circles between SourceMap and SourceMapFile
.filter(sourceMapCircleFilter)
.some(rel => rel.to.url.startsWith(assetGraph.root));

if (
Expand All @@ -119,7 +141,7 @@ module.exports = async function hashfiles(assetGraph, options = {}) {
.forEach(function(incomingRelation) {
if (cdnRoot.startsWith('//')) {
incomingRelation.hrefType = 'protocolRelative';
} else if (cdnRoot.startsWith('http')) {
} else if (asset.type === 'SourceMap') {
incomingRelation.hrefType = 'absolute';
}
// Set crossorigin=anonymous on <script> tags pointing at CDN JavaScript.
Expand Down
82 changes: 59 additions & 23 deletions test/hashfiles.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ async function getPopulatedGraph(root, entryPoints) {
await graph.loadAssets(entryPoints);
await graph.populate({
followRelations: {
crossorigin: false
crossorigin: false,
type: { $nin: ['JavaScriptFetch', 'SourceMapSource', 'SourceMapFile'] }
}
});

Expand Down Expand Up @@ -278,7 +279,12 @@ describe('hashfiles', () => {
{
origin: 'file://',
path: '/static/',
fileName: 'main.eac26617cf.js'
fileName: 'main.cd90e77c46.js'
},
{
origin: 'file://',
path: '/static/',
fileName: 'main.js.71801f227c.map'
},
{
origin: 'file://',
Expand Down Expand Up @@ -323,6 +329,56 @@ describe('hashfiles', () => {
]);
});

it('should put hashed sourcemaps /myCdnRoot', async () => {
const graph = new AssetGraph({
root: resolve(__dirname, '../testdata', 'sourcemapCdn')
});

await graph.loadAssets(['index.html']);
await graph.populate({
followRelations: {
crossorigin: false
}
});

await hashFiles(graph, { cdnRoot: 'https://mycdn.com' });

const allFileAssets = graph
.findAssets({
isLoaded: true,
isInline: false
})
.sort((a, b) => a.fileName.localeCompare(b.fileName));

expect(allFileAssets, 'to satisfy', [
{
origin: 'file://',
path: '/',
fileName: 'index.html'
},
{
origin: 'https://mycdn.com',
path: '/',
fileName: 'main.6955839b28.js'
},
{
origin: 'https://mycdn.com',
path: '/',
fileName: 'main.c1453512ae.coffee'
},
{
origin: 'https://mycdn.com',
path: '/',
fileName: 'main.js.07d32556c8.map'
}
]);

expect(graph, 'to contain relation', {
to: { type: 'SourceMap' },
hrefType: 'absolute'
});
});

it('should put crossorigin attributes on HtmlRelations to CDN assets', async () => {
const graph = await getPopulatedGraph('cdntest', ['index.html']);

Expand All @@ -340,50 +396,30 @@ describe('hashfiles', () => {
type: 'HtmlStyle',
href: 'https://mycdn.com/simple.829e4ff717.css',
crossorigin: true,
hrefType: 'absolute',
node: expect.it('to have attribute', 'crossorigin')
},
{
type: 'HtmlStyle',
href: 'static/style.58c0948f5e.css',
crossorigin: false,
hrefType: 'relative',
node: expect.it('not to have attribute', 'crossorigin')
},
{
type: 'HtmlScript',
href: 'https://mycdn.com/simple.3831d504d8.js',
crossorigin: true,
hrefType: 'absolute',
node: expect.it('to have attribute', 'crossorigin')
},
{
type: 'HtmlScript',
href: 'static/main.eac26617cf.js',
href: 'static/main.cd90e77c46.js',
crossorigin: false,
hrefType: 'relative',
node: expect.it('not to have attribute', 'crossorigin')
}
]
);
});

it('should have absolute hreftypes to CDN assets', async () => {
const graph = await getPopulatedGraph('cdntest', ['index.html']);

await hashFiles(graph, { cdnRoot: 'https://mycdn.com' });

expect(
graph.findRelations({
to: { origin: 'https://mycdn.com' }
}),
'to have items satisfying',
{
hrefType: 'absolute'
}
);
});

it('should have protocol-relative hreftypes to CDN assets', async () => {
const graph = await getPopulatedGraph('cdntest', ['index.html']);

Expand Down
17 changes: 15 additions & 2 deletions testdata/cdntest/main.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
fetch('/main.tpl.html').then(res => res.text().then(console.log));
fetch('/main.tpl.html'.toString('url')).then(res =>
res.text().then(console.log)
);

console.log('main script');
// Generated by CoffeeScript 2.3.0
(function() {
var square;

square = function(x) {
return x * x;
};

alert(square(10));
}.call(this));

// # sourceMappingURL=main.js.map
13 changes: 13 additions & 0 deletions testdata/cdntest/main.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions testdata/sourcemapCdn/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<!DOCTYPE html>
<html lang="en">

<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<meta http-equiv="X-UA-Compatible" content="ie=edge">
<title>Document</title>
<script src="main.js"></script>
</head>

<body>

</body>

</html>
1 change: 1 addition & 0 deletions testdata/sourcemapCdn/main.coffee
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log('this is not the real source');
3 changes: 3 additions & 0 deletions testdata/sourcemapCdn/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
console.log('this is not the real source');

// # sourceMappingURL=main.js.map
13 changes: 13 additions & 0 deletions testdata/sourcemapCdn/main.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 81b5bf4

Please # to comment.