Closed
Description
Description of Proposed Feature
Auto-import is a great feature we rely on to load our app inlined. However, in some cases, the retrieval of the current running script will not be correct, as the last script might not always be the running script. (https://github.com/systemjs/systemjs/blob/master/src/features/script-load.js#L40)
systemJSPrototype.register = function (deps, declare) {
if (hasDocument && document.readyState === 'loading' && typeof deps !== 'string') {
var scripts = document.getElementsByTagName('script');
var lastScript = scripts[scripts.length - 1]; // not always the running script
var url = lastScript && lastScript.src;
if (url) {
lastAutoImportUrl = url;This proposal would extract the retrieval of the current script into a separate, overridable, hook:
systemJSPrototype.getCurrentScript = function () {
var scripts = document.getElementsByTagName('script');
return scripts[scripts.length - 1];
};
systemJSPrototype.register = function (deps, declare) {
if (hasDocument && document.readyState === 'loading' && typeof deps !== 'string') {
var lastScript = systemJSPrototype.getCurrentScript();
var url = lastScript && lastScript.src;
if (url) {
lastAutoImportUrl = url;In what way would you use it?
In the end state, when we drop support for IE 11 in our app, we would use document.currentScript in order to always have the correct current script:
systemJSPrototype.getCurrentScript = function () {
return document.currentScript;
};In the meantime, we would use the hook to return the correct script element dynamically. It would also allow us to prevent the import of incorrectly matched last scripts when dynamic loads have been used.
Metadata
Metadata
Assignees
Labels
No labels
Activity
guybedford commentedon Sep 3, 2020
Thanks for posting. If it's never not correct that's a bug though, do you have any replications you can share?
guybedford commentedon Sep 3, 2020
Are you specifically seeing issues in IE11?
guybedford commentedon Sep 3, 2020
A stack-based error approach may make sense for an IE11 specific fix...
guybedford commentedon Sep 3, 2020
(ie using
document.currentScriptand falling back to a stack approach in IE11)But the timing approach as currently should work I believe, and the whole nice thing about it was avoiding the need for the above.
tmsns commentedon Sep 4, 2020
So, the issue happens both in IE11 and Chrome.
Our html page looks like this:
Using
document.currentScriptfixes the issue for non-IE browsers. :-) As IE has recently got a support end date for Microsoft 365 apps, it's probable IE support will be dropped somewhere next year as well. In the mean time, this proposal would allow to write some workaround code for IE.It's also important to note that this is a more complex integration scenario. Most usage of SystemJS is probably done on an empty page. In that case, the current IE-proof solution (to get the current script element using the last script element) is working correctly. This is the reason why we proposed this as a hook to be implemented to allow users to change this behavior if their use case requires it. (without breaking other users's setup with more complex internal solutions)
It's also a bit difficult to have a correct replication, as the external-script.js is extending the loading state of the document because of extra request/responses (so network related), but also with extra code of course. I will try however, to still reproduce this. But I cannot guarantee it will work.. :-/
guybedford commentedon Sep 4, 2020
There is a chance there could be a bug in what we previously implemented, so I would very much like to try to fix that first and foremost here. Thanks for trying to find a replication. If it's something you can only send privately we could arrange that as well.
Will await further feedback.
tmsns commentedon Sep 7, 2020
I was finally able to reproduce it! 😃
https://plnkr.co/edit/dMPuci6LJh0smbyu
Some remarks about the repro:
document.writeinindex.htmlis used in order to fake a sync third party library, but I wanted to use a file in the same project for it (animals/chicken.js)flash.siwalik.in(a https variant of Slowwly)triton.js). These dependencies are retrieved with a url that is constructed using the wrong base url. That wrong base url is coming fromdocument.getElementsByTagName('scripts')which retrieves the last script instead of the current script.Let me know if can help more!
guybedford commentedon Sep 11, 2020
Thanks so much @tmsns for the very clear replication here. I've posted a fix in the PR at #2245.
guybedford commentedon Sep 13, 2020
@tmsns this fix was released in 6.6.0 and 6.6.1, just let me know if you hit any further issues here or need to reopen.