Pamasich / kbin-kes

Add-on manager and scripting framework for kbin
MIT License
0 stars 0 forks source link

ESLint Issues #64

Open Pamasich opened 6 months ago

Pamasich commented 6 months ago

The main KES script and several of the mods have issues in eslint, especially the requirement for mods to have an entry function that goes unused in the mod file itself makes one such error practically guaranteed.

Try getting rid of those, if needed by adding exclusions where it makes sense. Check my own mods, where I already exclude the entry function from the unused rule.

The userscript metadata block currenlty has a lot of issues because of the line length of the raw urls. I think it makes sense to exclude the block from the line length rule since we can't really do much to control it there. Adding /* eslint-disable max-len */ before it starts and /* eslint-enable max-len */ after its last line should do the trick.

The globals maybe can be put on multiple lines? Avoid using exclusions if possible, only use them if there's no other option and if it makes sense to use them.

aclist commented 6 months ago

Technically, the functions are being exported into the global scope when the mod files are ingested, so to be more specific, the exported rule could be used, rather than ignoring the line generically. But this export could be listed inline in the mod instead of in KES. Could be done programmatically at "compile" time as well so that mod authors don't have to do it.

The thing is, the globals line in KES proper is essentially used to prevent ESLint from complaining about the function names programmatically inserted into the funcObj, which stores the names of all the entrypoint functions. This was being used to work around the limitation of needing to interpolate stringwise function names from the manifest and then access them as actual functions.

But thinking more radically, if switching to the namespacing logic, something like this basic prototype should work, and we could drop all of that unnecessary stuff from the masthead:

KES generator rolls atomic mods into one object...

const init = {
    fixCodeblocks: function(toggle){
        function enable(){
          console.log("enabling");
        }
        function disable(){
          console.log("disabling");
        }
        if (toggle):
            enable();
    }
    someOtherMod: function(toggle){
    ...
    }
}

KES loads the object at runtime...

When parsing the manifest, KES could just check for the mod name and then call init as:

//iterate through JSON
//...
let mod = json[i].name
init[mod](toggle)

No need to populate the names of mods ahead of time and no need to exclude them from ESLint as globals, since they are contained as methods of the init object and can be accessed using bracket notation by interpolating strings.

The mod author is just responsible for creating:

    function(toggle){
        function enable(){
          console.log("enabling");
        }
        function disable(){
          console.log("disabling");
        }
        if (toggle):
            enable();
    }

And KES generator wraps each mod by name as:

    fixCodeblocks: function(toggle){
        function enable(){
          console.log("enabling");
        }
        function disable(){
          console.log("disabling");
        }
        if (toggle){
            enable();
        }
    }

Before finally wrapping the entire thing in an object, as seen above.

Thoughts? This should take care of both this issue and the one discussed in #63.

Pamasich commented 6 months ago

Oh, that sounds like a good idea.

The mod author is just responsible for creating:

   function(toggle){
       function enable(){
         console.log("enabling");
       }
       function disable(){
         console.log("disabling");
       }
       if (toggle):
           enable();
   }

The code editor will complain though if the function with the toggle parameter doesn't have a name in the code. At least mine does. I don't know Javascript enough, would it matter with this proposed workflow if I named that function?

aclist commented 6 months ago

The code editor will complain though if the function with the toggle parameter doesn't have a name in the code. At least mine does. I don't know Javascript enough, would it matter with this proposed workflow if I named that function?

It won't matter if a name is given, calling the outer function by object key name will trigger the enclosing function regardless. You can name it whatever you want.

Does it seem reasonable to use the names of the mod directories as the qualified name for the object keys?

Pamasich commented 6 months ago

Does it seem reasonable to use the names of the mod directories as the qualified name for the object keys?

I think that would be ideal. The keys need to be unique and the directory names are guaranteed to be unique.

aclist commented 6 months ago

Does it seem reasonable to use the names of the mod directories as the qualified name for the object keys?

I think that would be ideal. The keys need to be unique and the directory names are guaranteed to be unique.

Yes, we only need to check if incoming PRs are trying to use a pre-existing directory name for the mod name, but that should be almost impossible or highly unlikely.

aclist commented 5 months ago

I haven't yet thought of a programmatic way of excluding the outer function in a new mod from triggering no-unused-vars, nor do I know if this should be something that's done programmatically.