Add modulesWithSideEffectsInSrcFiles to allow esm in srcFiles

[HolgerJeromin]

We have some ES modules which needs to be loaded as spec files.

[sgravrock]

Can you help me understand why this is useful? Needing ES modules loaded that you never import seems quite unusual. A link to a repo with a minimal reproducible example of the problem you're trying to solve might help.

I'm not necessarily opposed to this feature, but I do think the name will cause confusion. I think most users would reasonably assume they need allowModuleSrcFiles set to true if they have ES modules in the src directory, but it should actually be set to false.

[HolgerJeromin]

Needing ES modules loaded that you never import seems quite unusual.

This is useful when the module code has side effects.
The easiest example for example is having a polyfill delivered by module. Its code needs to be loaded before script srcFiles.

But there are more complex scenarios where script and modules code needs even to be interleaved.
My very usecase is migrating to modules file by file. We are based on script code and switching to modules but need to support script code which is depending on our base.

Complex minimal example

// <script src=baseinfrastructure.js defer>
function registerControls(){ }

// <script src=basecontrol.js defer>
class control{ /* much code */}
registerControls(control);

// <script src=interactionControl.mjs type=module>  migrated code
class interactionControl extends control{ /* much code */}
registerControls(interactionControl);
window.interactionControl = interactionControl; // compatibility layer

// <script src=button.js defer> old code
class button extends interactionControl{ /* much code */}
registerControls(button);

A further problem is that you have to use enableTopLevelAwait to load js code sequentially. This is because the wrapper _jasmine_loadEsModule kills the loading order. This came in 4 years ago in f0ca479f662cf473822915d218b4036ce764a6b2 to fight a safari bug.

I'm not necessarily opposed to this feature, but I do think the name will cause confusion.

Yeah you are right.
Will try to find a better name.

[HolgerJeromin]

@sgravrock
Can you estimate if the referenced safari bug is still relevant?
run.html.ejs with this snipped works like a charm and adds my class or a polyfill without enableTopLevelAwait:

    <% userJsFiles.forEach(function(jsFile) { %>
      <% if (jsFile.endsWith(esmFilenameExtension)) { %>
        <script type="module" src="<%= jsFile %>"></script>
     <% } else if (moduleWithSideeffectsInSrcFiles) { %>
          <script src="<%= jsFile %>" type="text/javascript" defer></script>
      <% } else { %>
        <script src="<%= jsFile %>" type="text/javascript"></script>
      <% } %>
    <% }) %>

[sgravrock]

Can you estimate if the referenced safari bug is still relevant?

Yes, but not right away. I don't currently have a machine that runs Safari 15, so I'd have to rig something up to run on Saucelabs. Not that big a deal but it'll have to wait for the next time when I have a solid block of time for Jasmine.

 <% } else if (moduleWithSideeffectsInSrcFiles) { %>
      <script src="<%= jsFile %>" type="text/javascript" defer></script>

Why defer?

[HolgerJeromin]

Why defer?

Because it is executed at the exact same timing as modules which are also loaded deferred.
So each file is executed (after the document has been parsed and) after the line before:

<script src="a.js" defer></script>
<script src="b.js" type=module></script>
<script src="c.js" defer></script>

Which is crucial for having the side effect effective.

[HolgerJeromin]

Not that big a deal but it'll have to wait for the next time when I have a solid block of time for Jasmine.

That would be awesome.
Removing that hack would make the jasmine-browser-runner code much easier to understand and allow more scenarios out of the box (without the enableTopLevelAwait timing "hack").

[sgravrock]

| Can you estimate if the referenced safari bug is still relevant?

Looks like it's still needed. The bug was fixed in Safari 16, but Jasmine still supports Safari 15.

I'm considering a change to the browser support policy to deal with the difficulty of testing on multiple versions of Safari. But as of today Safari 15 is a fully supported browser, and I'd want to wait as long as practical after a policy change before actually shipping anything that breaks it.

[sgravrock]

This also needs a jsdoc update and tests. Given the nature of the change, I'd recommend an integration test that actually runs jasmine-browser-runner with a suite that would fail if the feature didn't work. See esmIntegrationSpec.js for an example. Supporting unit tests are also welcome.

[HolgerJeromin]

Looks like it's still needed. The bug was fixed in Safari 16, but Jasmine still supports Safari 15.

Ok. Thank you very much for the test.

Will create a unit test for that in the next week.

I will try to document the need of this new setting.

[HolgerJeromin]

I added the the documentation and a test. I am unsure if it needs more hints that one might need enableTopLevelAwait.
Would be happy to hear your feedback.

[sgravrock]

Looks reasonable at a glance, I just haven't had time to take a decent look at it yet. Stay tuned.

[HolgerJeromin]

I am unsure if I need to mention enableTopLevelAwait (for sequencial source loading) here, too.

It is needed but can be confusing.

[sgravrock]

| Can you estimate if the referenced safari bug is still relevant?

Looks like it's still needed. The bug was fixed in Safari 16, but Jasmine still supports Safari 15.

I'm considering a change to the browser support policy to deal with the difficulty of testing on multiple versions of Safari. But as of today Safari 15 is a fully supported browser, and I'd want to wait as long as practical after a policy change before actually shipping anything that breaks it.

Actually the bug affects Safari 16 as well. But as far as I can tell Safari 16 has reached end of life. While I'd rather not drop it entirely, I think it would be ok to start removing workarounds for its ESM bugs. If I fixed #62 by emitting script tags in the HTML rather than using _jasmine_loadEsModule, would that allow you to simplify things?

I think I can do that regardless of the outcome of jasmine/jasmine#2050 .

[HolgerJeromin]

If I fixed #62 by emitting script tags in the HTML rather than using _jasmine_loadEsModule, would that allow you to simplify things?

Yes. I had locally removed _jasmine_loadEsModule and it worked like a charm.
This would make my modulesWithSideEffectsInSrcFiles option much easier to use and understand.

I could create a PR (which removes _jasmine_loadEsModule) in the next days. But I understand if you want to to refactor such a "big" / "core" change by yourself. I would be happy to rebase this PR onto your work.