Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call fetch hook when retrieving external import maps. Resolves #2374. #2376

Merged
merged 3 commits into from
Jan 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ git:
depth: 1
language: node_js
node_js:
- node
# ncc doesn't work with node 17 yet
- 16
env:
global:
- MOZ_HEADLESS=1
Expand Down
2 changes: 1 addition & 1 deletion docs/hooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ System.shouldFetch = function () { return true; };

will enforce loading all JS files through `fetch`, allowing custom fetch hook implementation behaviours.

#### fetch(url) -> Promise<Response>
#### fetch(url, options) -> Promise<Response>

The default fetch implementation used in SystemJS is simply `System.fetch = window.fetch`, which can be further hooked to enable
arbitrary transformation.
Expand Down
3 changes: 3 additions & 0 deletions src/extras/module-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ import { resolveUrl } from '../common.js';
systemJSPrototype.fetch = function (url, options) {
return fetch(url, options)
.then(function (res) {
if (options.passThrough)
return res;

if (!res.ok)
return res;
var contentType = res.headers.get('content-type');
Expand Down
3 changes: 2 additions & 1 deletion src/features/import-maps.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ function processScripts () {
}
else if (script.type === 'systemjs-importmap') {
script.sp = true;
var fetchPromise = script.src ? fetch(script.src, { integrity: script.integrity }).then(function (res) {
// The passThrough property is for letting the module types fetch implementation know that this is not a SystemJS module.
var fetchPromise = script.src ? (System.fetch || fetch)(script.src, { integrity: script.integrity, passThrough: true }).then(function (res) {
if (!res.ok)
throw Error(process.env.SYSTEM_PRODUCTION ? res.status : 'Invalid status code: ' + res.status);
return res.text();
Expand Down
27 changes: 27 additions & 0 deletions test/browser/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,33 @@ suite('SystemJS Standard Tests', function() {
);
});

test('Calls the fetch hook when fetching import maps', function () {
const importMapSrc = 'https://example.com/importmap-test.js';
const scriptElement = document.createElement('script');
scriptElement.type = 'systemjs-importmap';
scriptElement.src = importMapSrc;
document.head.appendChild(scriptElement);

const originalFetch = System.constructor.prototype.fetch;

let fetchHookCalled = false;

System.constructor.prototype.fetch = function(url) {
if (url === importMapSrc) {
fetchHookCalled = true;
}

return originalFetch.apply(this, arguments);
}

// Reprocess import maps
System.prepareImport(true);

System.constructor.prototype.fetch = originalFetch;

assert.ok(fetchHookCalled);
});

var isIE11 = typeof navigator !== 'undefined' && navigator.userAgent.indexOf('Trident') !== -1;

function isCSSStyleSheet(obj) {
Expand Down