How to detect circular imports in a custom DocumentLoader?

[MF91WorkPS]

Hi,

we use a custom DocumentLoader to import scripts from different locations. Recently, one of our Test-Apps crashed because of a StackOverflowException when calling V8ScriptEngine.Evaluate(script). Our analysis revealed that the cause was a circular import defined by one of our users. We were able to reproduce the issue using a simplified example:

main.js
import { something } from 'testModule';
testModule.js
import { something } from 'testModule2';
testModule2.js
import { something } from 'testModule';

Obviously, we do not want our app to crash when a user makes a mistake and defines a circular import. We are considering to limit the number of allowed imports to prevent circular imports:

using Microsoft.ClearScript;
using Microsoft.ClearScript.JavaScript;
using System;
using System.Collections.Generic;
using System.Threading.Tasks;

namespace CompanyName.ProductName.JavaScript.DocumentLoading
{
    //NOTE: Exception handling and script loading removed/simplified.
    public class CustomDocumentLoader : DocumentLoader
    {
        private List<string> _loadedScripts = new List<string>();

        public override Document LoadDocument(
            DocumentSettings settings,
            DocumentInfo? sourceInfo,
            string fileName,
            DocumentCategory category,
            DocumentContextCallback contextCallback)
        {
            if (string.IsNullOrWhiteSpace(fileName))
                throw new Exception($"The import statement is invalid. '{fileName}' is not a valid file name.");

            _loadedScripts.Add(fileName);

            if (_loadedScripts.Count >= 1000)
            {
                throw new Exception(
                    "Maximum number of allowed imports reached. Potential circular import detected. " +
                    $"Previously loaded scripts are: {string.Join(",", _loadedScripts)}.");
            }

            try
            {
                return LoadDocumentAsync(settings, sourceInfo, fileName, category, contextCallback).Result;
            }
            catch (AggregateException exception)
            {
                throw new Exception($"Executing an import statement failed. Error on loading the script with the name '{fileName}': {exception}.");
            }
        }

        public override async Task<Document> LoadDocumentAsync(
            DocumentSettings settings, 
            DocumentInfo? sourceInfo, 
            string specifier, 
            DocumentCategory category, 
            DocumentContextCallback contextCallback)
        {
            return await Task.Run(() =>
            {
                //Custom script loading logic.

                return new StringDocument(new DocumentInfo("someScriptname") { Category = ModuleCategory.Standard }, "someContent");
            });
        }
    }
}

However, we are worried that the approach may not work in all cases. Our current solution would also limit the number of import statements to 1000 which may be problematic in the future since our user constantly extend their solutions. We would like to give more information to the user about the cause of the circular import. Can give some advice on what might be the best way to detect circular imports? Thanks!

[ClearScriptLib]

Hi @MF91WorkPS,

As far as we're aware, there's nothing unusual or incorrect about module cycles. Standard and CommonJS modules have strong identities that ensure uniqueness. That is, once you've imported a given module, subsequent imports of the same module provide access to the same actual module instance.

Also, strictly speaking, the document loader has nothing to do with modules or imports. It's just a way to load scripts or other documents from the file system, the web, or elsewhere. However, documents, like modules, must also have strong identities, and the loader must ensure that a given document is only loaded once.

That's documented in ClearScript's API reference for LoadDocument and LoadDocumentAsync:

"A loaded document must have an absolute URI. Once a load operation has completed successfully, subsequent requests that resolve to the same URI are expected to return the same Document reference ..."

Complying with this requirement should prevent issues such as the one you're encountering. A simple way might be to track loaded documents via a string-to-Document or URI-to-Document dictionary, which is essentially what ClearScript's default loader does. In fact, your custom loader can defer to the default loader for caching purposes.

Good luck!

[MF91WorkPS]

Hi, I did not know that modules cycles were allowed. We were not using caching before. I implemented a cache similar to the default cache and it works. Thanks!