-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
module-types use assert type to detect module type #2434
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -12,12 +12,22 @@ SystemJS supports loading modules that are in the following formats: | |||||
| [AMD](https://github.com/amdjs/amdjs-api/wiki/AMD) | [AMD extra](/dist/extras/amd.js) | [AMD extra](/dist/extras/amd.js) | * | | ||||||
| [UMD](https://github.com/umdjs/umd) | [AMD extra](/dist/extras/amd.js) | [AMD extra](/dist/extras/amd.js) | * | | ||||||
|
||||||
### File Extension Limitations | ||||||
### Module type detection | ||||||
|
||||||
When loading JSON modules, CSS modules and Web Assembly modules, the browser specifications require interpreting these modules based on checking their MIME type. Since SystemJS has to choose upfront whether to append a script element (for JS modules) or make a fetch request (for a JSON/CSS/Wasm module), it needs to know the module type upfront at resolution time. | ||||||
|
||||||
Instead of reading the MIME type, the file extension is thus used specifically for the JSON, CSS and Web Assembly module cases. | ||||||
|
||||||
If module type is specified by import, the mime is acceptable as the module type, will use the specified module type. | ||||||
|
||||||
e.g. | ||||||
|
||||||
```js | ||||||
System.import('https://example.com/my-module.txt', {assert: {type: 'css'}}); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
``` | ||||||
|
||||||
This will force resolve the module as a CSS module, even if the file extension is `.txt` and the MIME type is `text/plain`. | ||||||
|
||||||
## JSON Modules | ||||||
|
||||||
[JSON modules](https://github.com/whatwg/html/pull/4407) support importing a JSON file as the default export. | ||||||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -1,4 +1,5 @@ | ||||||||||
import { resolveUrl } from '../common.js'; | ||||||||||
import {errMsg} from '../err-msg.js'; | ||||||||||
|
||||||||||
/* | ||||||||||
* Loads JSON, CSS, Wasm module types based on file extension | ||||||||||
|
@@ -9,35 +10,32 @@ import { resolveUrl } from '../common.js'; | |||||||||
|
||||||||||
var moduleTypesRegEx = /^[^#?]+\.(css|html|json|wasm)([?#].*)?$/; | ||||||||||
var _shouldFetch = systemJSPrototype.shouldFetch.bind(systemJSPrototype) | ||||||||||
systemJSPrototype.shouldFetch = function (url) { | ||||||||||
return _shouldFetch(url) || moduleTypesRegEx.test(url); | ||||||||||
systemJSPrototype.shouldFetch = function (url, parent, meta) { | ||||||||||
var assertType = ((meta || {}).assert || {}).type; | ||||||||||
return _shouldFetch(url, parent, meta) || moduleTypesRegEx.test(url) || !!loaders[assertType]; | ||||||||||
Comment on lines
+14
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
}; | ||||||||||
|
||||||||||
var jsonContentType = /^application\/json(;|$)/; | ||||||||||
var cssContentType = /^text\/css(;|$)/; | ||||||||||
var wasmContentType = /^application\/wasm(;|$)/; | ||||||||||
|
||||||||||
var fetch = systemJSPrototype.fetch; | ||||||||||
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'); | ||||||||||
if (jsonContentType.test(contentType)) | ||||||||||
return res.json() | ||||||||||
var fallback = '_fallback'; | ||||||||||
var contentTypes = [ | ||||||||||
[/^application\/json(;|$)/, 'json'], | ||||||||||
[/^text\/css(;|$)/, 'css'], | ||||||||||
[/^application\/wasm(;|$)/, 'webassembly'], | ||||||||||
[/^text\/plain(;|$)/, fallback], | ||||||||||
[/^application\/octet-stream(;|$)/, fallback], | ||||||||||
] | ||||||||||
var loaders = { | ||||||||||
'json': function (res) { | ||||||||||
return res.json() | ||||||||||
.then(function (json) { | ||||||||||
return new Response(new Blob([ | ||||||||||
'System.register([],function(e){return{execute:function(){e("default",' + JSON.stringify(json) + ')}}})' | ||||||||||
], { | ||||||||||
type: 'application/javascript' | ||||||||||
})); | ||||||||||
}); | ||||||||||
if (cssContentType.test(contentType)) | ||||||||||
return res.text() | ||||||||||
}) | ||||||||||
}, | ||||||||||
'css': function (res, url) { | ||||||||||
return res.text() | ||||||||||
.then(function (source) { | ||||||||||
source = source.replace(/url\(\s*(?:(["'])((?:\\.|[^\n\\"'])+)\1|((?:\\.|[^\s,"'()\\])+))\s*\)/g, function (match, quotes, relUrl1, relUrl2) { | ||||||||||
return 'url(' + quotes + resolveUrl(relUrl1 || relUrl2, url) + quotes + ')'; | ||||||||||
|
@@ -48,11 +46,11 @@ import { resolveUrl } from '../common.js'; | |||||||||
type: 'application/javascript' | ||||||||||
})); | ||||||||||
}); | ||||||||||
if (wasmContentType.test(contentType)) | ||||||||||
return (WebAssembly.compileStreaming ? WebAssembly.compileStreaming(res) : res.arrayBuffer().then(WebAssembly.compile)) | ||||||||||
}, | ||||||||||
'webassembly': function (res, url) { | ||||||||||
return (WebAssembly.compileStreaming ? WebAssembly.compileStreaming(res) : res.arrayBuffer().then(WebAssembly.compile)) | ||||||||||
.then(function (module) { | ||||||||||
if (!global.System.wasmModules) | ||||||||||
global.System.wasmModules = Object.create(null); | ||||||||||
if (!global.System.wasmModules) global.System.wasmModules = Object.create(null); | ||||||||||
global.System.wasmModules[url] = module; | ||||||||||
// we can only set imports if supported (eg early Safari doesnt support) | ||||||||||
var deps = []; | ||||||||||
|
@@ -65,15 +63,46 @@ import { resolveUrl } from '../common.js'; | |||||||||
setterSources.push('function(m){i[' + key + ']=m}'); | ||||||||||
} | ||||||||||
}); | ||||||||||
return new Response(new Blob([ | ||||||||||
'System.register([' + deps.join(',') + '],function(e){var i={};return{setters:[' + setterSources.join(',') + | ||||||||||
'],execute:function(){return WebAssembly.instantiate(System.wasmModules[' + JSON.stringify(url) + | ||||||||||
'],i).then(function(m){e(m.exports)})}}})' | ||||||||||
], { | ||||||||||
return new Response(new Blob(['System.register([' + deps.join(',') + '],function(e){var i={};return{setters:[' + setterSources.join(',') + '],execute:function(){return WebAssembly.instantiate(System.wasmModules[' + JSON.stringify(url) + '],i).then(function(m){e(m.exports)})}}})'], { | ||||||||||
type: 'application/javascript' | ||||||||||
})); | ||||||||||
}); | ||||||||||
return res; | ||||||||||
}); | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
var fetch = systemJSPrototype.fetch; | ||||||||||
systemJSPrototype.fetch = function (url, options) { | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I missed that we were passing the assertion directly as |
||||||||||
return fetch(url, options) | ||||||||||
.then(function (res) { | ||||||||||
if (options.passThrough) return res; | ||||||||||
|
||||||||||
if (!res.ok) return res; | ||||||||||
|
||||||||||
var assertType = ((options.meta || {}).assert || {}).type; | ||||||||||
var mime = res.headers.get('content-type'); | ||||||||||
var contentType | ||||||||||
for (var i = 0; i < contentTypes.length; i++) { | ||||||||||
var rule = contentTypes[i]; | ||||||||||
if (rule[0].test(mime)) { | ||||||||||
contentType = rule[1] | ||||||||||
break | ||||||||||
} | ||||||||||
} | ||||||||||
Comment on lines
+82
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if there's a shorter way to do this? |
||||||||||
|
||||||||||
// ensure type match | ||||||||||
if (assertType && contentType){ | ||||||||||
if(contentType === fallback){ | ||||||||||
contentType = assertType | ||||||||||
} | ||||||||||
if (assertType !== contentType){ | ||||||||||
console.warn(process.env.SYSTEM_PRODUCTION ? errMsg('W7') : errMsg('W7', `Module ${url} has an unexpected content type of "${mime}": expected module type ${assertType} got ${contentType}.`)); | ||||||||||
contentType = undefined | ||||||||||
} | ||||||||||
} | ||||||||||
Comment on lines
+93
to
+101
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems fine to me, just watch the spacing of the syntax. Also shorter code is always better for this lib! |
||||||||||
|
||||||||||
var loader = loaders[contentType]; | ||||||||||
if (loader) return loader(res, url, options); | ||||||||||
return res; | ||||||||||
}); | ||||||||||
}; | ||||||||||
})(typeof self !== 'undefined' ? self : global); |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -227,6 +227,22 @@ suite('SystemJS Standard Tests', function() { | |||||||||
}); | ||||||||||
}); | ||||||||||
|
||||||||||
test('should load a.txt as css module by assert', function () { | ||||||||||
return System.import('fixturesbase/css-modules/a.txt', {assert: {type: 'css'}}).then(function (m) { | ||||||||||
Comment on lines
+230
to
+231
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
assert.ok(m); | ||||||||||
assert.ok(isCSSStyleSheet(m.default)); | ||||||||||
document.adoptedStyleSheets = document.adoptedStyleSheets.concat(m.default); | ||||||||||
}); | ||||||||||
}); | ||||||||||
|
||||||||||
test('should load a as css module by assert', function () { | ||||||||||
return System.import('fixturesbase/css-modules/a', {assert: {type: 'css'}}).then(function (m) { | ||||||||||
Comment on lines
+238
to
+239
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
assert.ok(m); | ||||||||||
assert.ok(isCSSStyleSheet(m.default)); | ||||||||||
document.adoptedStyleSheets = document.adoptedStyleSheets.concat(m.default); | ||||||||||
}); | ||||||||||
}); | ||||||||||
|
||||||||||
test('should support application/javascript css module override', function () { | ||||||||||
return System.import('fixturesbase/css-modules/javascript.css').then(function (m) { | ||||||||||
assert.ok(m); | ||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
.hello { | ||
background-color: peru; | ||
} | ||
body { | ||
background-color: lightblue; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
.hello { | ||
background-color: peru; | ||
} | ||
body { | ||
background-color: lightblue; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.