Handle inline systemjs module scripts #2357
Conversation
File size impactMerging main into main will impact 3 files in 2 groups. browser (2/2)
node (1/1)
extras (0/8)No impact on files in extras group. |
There was a problem hiding this comment.
After looking through this, my first thought was "why not just use <script type="text/javascript">?" Is it because you wish to be able to import from other modules? I'd like to understand the use case more - the example you shared doesn't call System.register() nor does it import or export anything inside of the inline module, so it's unclear to me why you couldn't just use <script type="text/javascript">.
One thing to note is that with native modules it is not possible to retrieve the exports of inlined modules. For example in the following code, it's not possible to retrieve the a export in any way from any other module:
<script type="module">
export const a = 1;
</script>It's important to keep module script inline in the HTML file so that user sees the "hello world" message right away.
<script type=module> is always asynchronous, no matter what. So while inlining could have performance benefits by avoiding the network request to download the javascript file, it wouldn't ensure that the javascript is executed before the rest of the DOM content that comes below the <script>. This is not a reason to dismiss the feature entirely, but I just wanted to note it since I think any systemjs implementation should mimic that async behavior.
| if (typeof inlineScriptContent === "string") { | ||
| if (inlineScriptContent.indexOf("//# sourceURL=") < 0) | ||
| inlineScriptContent += "\n//# sourceURL=" + url | ||
| ;(0, eval)(inlineScriptContent) |
There was a problem hiding this comment.
If I understand things correctly, your code here requires the user to call System.import('https://example.com/_inline_script_0') in order for the <script type="systemjs-module"></script> to be executed? If so, I don't think it's a good solution for the following reasons:
- The
_inline_script_0part is a magic string that would not be obvious for people learning systemjs - Correlating an inline module to a URL is not something that browsers do and, as far as I know, there's no precedent for it.
- The user would probably expect the inline module to be executed automatically, and be confused when it isn't and requires them to call
System.import('https://example.com/_inline_script_0')manually to execute it.
There was a problem hiding this comment.
your code here requires the user to call System.import('https://example.com/_inline_script_0')
The code is auto executed when systemjs collects the script tags and calls System.import
systemjs/src/features/import-maps.js
Line 45 in 4da6ac7
There was a problem hiding this comment.
Ah I missed that, makes sense.
At first I was using an inline regular script tag. Then I wanted to import a module script from that regular script. So I naturally converted the regular script into a module one. Let me illustrate with code below. (I have shortened the example below to show the concept, in reality the code doing <!DOCTYPE html>
<html lang="en">
<head>
<title>Title</title>
<link rel="modulepreload" href="./app.js" />
</head>
<body>
<div id="app"></div>
<script type="module">
const app = document.querySelector('#app')
app.innerHTML = 'loading...'
try {
const { renderApp } = await import("./app.js")
app.innerHTML = 'loaded'
renderApp(app)
} catch(e) {
app.innerHTML = 'An error occured'
}
</script>
</body>
</html>I convert HTML into the following: <!DOCTYPE html>
<html lang="en">
<head>
<title>Title</title>
<script>
/* Inlined version of s.min.js */
</script>
<link rel="preload" href="./app.js" as="script" />
</head>
<body>
<script type="systemjs-module">
System.register([], function (exports, module) {
'use strict';
return {
execute: async function execute() {
const app = document.querySelector('#app')
app.innerHTML = 'loading...'
try {
const { renderApp } = await module.import("./app.js")
app.innerHTML = 'loaded'
renderApp(app)
} catch(e) {
app.innerHTML = 'An error occured'
}
}
}
})
</script>
</body>
</html>(In reality async/await is even converted to a promise based version but it's not important and would hurt readability) I think I have to use a module script because I want to import from a module. I don't think a regular script tag can do that in a clean way. One last thing: when browser support is good enough I am using the source files not the systemjs version. |
There was a problem hiding this comment.
In the example above, you could have just used System.import('./app.js') rather than module.import('./app.js'), so I don't think that situation is one that requires this feature.
According to the code below, we're going to be deprecating systemjs-module, so I don't think it makes sense to extend the feature further:
systemjs/src/features/import-maps.js
Line 31 in 98609db
The reason that we are planning on deprecating it is that auto-imports already automatically happen for systemjs modules that are in regular script tags. I've created https://codesandbox.io/s/intelligent-mahavira-jzkgk?file=/index.html which shows that inline systemjs modules already work in <script type="text/javascript">, so it's unclear to me why <script type="systemjs-module"> would be needed.
| if (typeof inlineScriptContent === "string") { | ||
| if (inlineScriptContent.indexOf("//# sourceURL=") < 0) | ||
| inlineScriptContent += "\n//# sourceURL=" + url | ||
| ;(0, eval)(inlineScriptContent) |
There was a problem hiding this comment.
Ah I missed that, makes sense.
|
I was not aware of the auto import feature and how it works 🤔 The behaviour I would like to have from systemjs is the following: <!DOCTYPE html>
<html lang="en">
<head>
<title>Title</title>
</head>
<body>
<!-- Not executed by SystemJS -->
<script>
console.log('Hello')
</script>
<script src="./jquery.js" />
<!-- Executed by Systemjs -->
<script type="systemjs-module">
function helper() {
console.log("Inline module executed!");
}
System.register([], () => ({
execute() {
helper()
}
}));
</script>
<script type="systemjs-module" src="./module.js"></script>
</body>
</html>Without |
You mean I could write the <script>
(async function execute() {
const app = document.querySelector('#app')
app.innerHTML = 'loading...'
try {
const { renderApp } = await System.import("./app.js")
app.innerHTML = 'loaded'
renderApp(app)
} catch(e) {
app.innerHTML = 'An error occured'
}
})()
</script>It would work indeed. |
SystemJS uses normal script tags to tell the browser to execute code, by creating
Yes your code example works.
I suggest converting the |
|
Now I get it, systems JS let browser do the job instead of systems module. |
|
According to my tests it works when systemjs is loaded with a script tag ( However systemjs code won't auto import an inline script when it's inline in the html file. systemjs/src/features/script-load.js Lines 38 to 41 in 6a5a620 |
|
I could make it work with the following changes (but I'm not sure these changes are correct, especially because I don't understand the need for setTimeout). + var inlineScriptCount = 0
systemJSPrototype.register = function (deps, declare) {
if (hasDocument && document.readyState === 'loading' && typeof deps !== 'string') {
var scripts = document.querySelectorAll('script[src]');
var lastScript = scripts[scripts.length - 1];
+ lastAutoImportDeps = deps;
if (lastScript) {
lastAutoImportUrl = lastScript.src;
- lastAutoImportDeps = deps;
- // if this is already a System load, then the instantiate has already begun
- // so this re-import has no consequence
- var loader = this;
- lastAutoImportTimeout = setTimeout(function () {
- autoImportCandidates[lastScript.src] = [deps, declare];
- loader.import(lastScript.src);
- });
}
+ else {
+ inlineScriptCount++
+ lastAutoImportUrl = document.location.href + "__inline_script__" +inlineScriptCount ;
+ }
+ // if this is already a System load, then the instantiate has already begun
+ // so this re-import has no consequence
+ var loader = this;
+ lastAutoImportTimeout = setTimeout(function () {
+ autoImportCandidates[lastAutoImportUrl] = [deps, declare];
+ loader.import(lastAutoImportUrl);
+ });
} |
|
Regarding inlining the systemjs library itself, please create a separate github issue, as that seems unrelated to this pull request. I'm closing this pull request since the functionality already works using |
|
Will do 🙏 |
Description
Makes Systemjs executes inline
<script type="systemjs-module">Motivation
I have an html file with inline module script. It's important to keep module script inline in the HTML file so that user sees the "hello world" message right away.
With this PR the following HTML would be executed by Systemjs.
Alternative
Could be replaced by
But it means an additional HTTP request performed by Systemjs to fetch
"inline_script.js". This would defeat the reason I inlined the script in the first place: show something to the user immediatly.See #2148 (comment)