search

comunica / sparqlee

⚙️ SPARQL expression evaluator library - Moved to @comunica/expression-evaluator
https://www.npmjs.com/package/sparqlee
14 stars 7 forks source link

Enable `no-implicit-globals` rule #95

Closed jitsedesmet closed 3 years ago

jitsedesmet commented 3 years ago

Trying to fix issue #93 I stumbled upon the fact that we use a lot of globally declared functions and maybe some variables (didn't check). This issue aims to remove all those global functions an variables. This issue does not cover global variables within the test folder.

wschella commented 3 years ago

This is only relevant in the browser I think? Should this not be okay in a ES module? https://eslint.org/docs/rules/no-implicit-globals

jitsedesmet commented 3 years ago

Looks like you're right. I don't know tho, @rubensworks asked me to fix them. What do you think @rubensworks ?

rubensworks commented 3 years ago

As sparqlee is written in a very functional style, keeping no-implicit-globals disabled does indeed make sense.

I would be a proponent of making sparqlee more object-oriented and composition-based though, as this would fit better in the overall architecture of Comunica. But that's something for the long-term though :-) As this definitely goes beyond this issue, I'm fine with closing it.