Added type stubs

[thatfloflo]

I've added type stubs covering the vast majority of the Python side of the eel API.

I've chosen to use the more legacy List, Dict, etc. from the typing package rather than paramterized builtins to maximise support for earlier python versions (e.g. Python 3.6).

There were some occasions where the type annotation is probably too generous, but where it was either impractical to type precisely (e.g. _ws, and in the browser files get_path seem to be inconsistent) or not readily apparent what is appropriate. Apart from get_path() which is clear on a module-by-module basis, I've annotated these with typing.Any for now, but of course it might be good to narrow them further going forward.

Tested and seem to work for me with pyright in vscode. Can't imagine any issues due to the relative simplicity of this, but I've not tested the stubs with other typecheckers.

[samuelhwilliams]

Hey @thatfloflo - thanks for the comprehensive type stubs!

Just wondering - what is the benefit of having these as separate .pyi files rather than adding the type hints into our .py files directly?

[thatfloflo]

@samuelhwilliams to be honest, I think it would be preferable to have inline annotations for the type hints, which I think makes for a cleaner package layout and is more maintainable. The only potential downside is a probably negligible hit on module load/startup times. However this has been targeted in repeated optimization since Python 3.7 so really shouldn't be an issue.

The reason I chose stubs for the PR at the time was twofold: we needed these on a project and could easily package the stubs and distribute them internally while still relying on the regular PyPI package for eel; and there had been many outstanding PRs for eel at the time, so that I thought it might be less troublesome to integrate this with other potential changes that might get pulled.

I'd be more than happy to go over this and turn them into inline annotations (probably also add a PEP 561 py.typed) if you're happy with that?

[samuelhwilliams]

@thatfloflo I would be very grateful if you could do that - thanks!

I totally agree and accept responsibility for the slow (almost non existant) handling of PRs and sorry for that. We've added a new contributor which should be mean things are a little bit more active now.

I'll review this PR again and get it merged once you've made those changes

[thatfloflo]

I've inlined the type hints now and both pytest/tox and mypy --strict are passing for me. The changes were a bit more involved than expected, but overall have been an improvement I think.

Summary:

• Many types have been narrowed further. There remain a few Any type hints where I'm wasn't confident what the appropriate narrowest type would be (e.g. _exposed_functions in init.py). These could potentially be narrowed further in the future, either by someone who knows the internals better or by runtime introspection
• Making the inline type hints compatible with Python 3.6 forced some slightly awkward solutions in places, mainly because we cannot from __future__ import annotations, so that the types that are annotated have to be available at runtime. These are isolated in a typing.TYPE_CHECKING branch however, so shouldn't have any noticeable runtime cost (typing.TYPE_CHECKING is always False at runtime).
• The jinja2 import is conditional in init.py:start(), but then later relied on without import check in init.py:_static(), so that there is a branch that is potentially available inside _static() which relies on a type that may not be available at runtime (e.g. if _start_args['jinja_env'] was set to some non-falsy value before _static() is called. This certainly isn't ideal, and means the Environment type is not deterministically available in the branch it is relied on (principally where template and response are assigned inside init.py:_static(). I've worked around this via a typing.TYPE_CHECKING conditional import in types.py and added a #typing.ignore inside init.py:_static(), since anything else would involve a more significant refactor of init.py.
• _start_args and the various options arguments use a dictionary with very complex possibilities for the type of the values. I've tried to piece together the possible range of types here from documentation, code analysis and a little but of runtime introspection, and think I've got the right now. However, it strikes me that something like a typing.NamedTuple or a dataclass would be a much better option for these, since they fully enumerate the options and can store specific types for each option. This would significantly reduce the margin for error in terms of the option values, but for users and automatic type checking. However, I've not made any such changes as they would likely lead to some backward incompatibilities, even if done very carefully.
• In a few cases, the situation with _start_args and options dictionaries above has led to insufficiently narrowable types. I've added type guards in these situations, which now raise a TypeError when an incompatible type is specified for these options. I'd say that's probably a good thing which makes eel more user-friendly, but of course adds some runtime cost. These can be removed if the _start_args/options arguments are ever moved to a NamedTuple or dataclass in the future.
• The type hints should cover all of the public and private first level constructs across eel now, but within functions not everything is fully annotated. This could be improved upon incrementally as and when sections of the code are updated, if desirable.
• I've added a py.typed in line with PEP 561.
• I've added a mypy.ini to suppress warnings for missing type information/stubs from external packages which do not currently have them. Some of these do now have typing information, but would require the dependency to be updated. So there's the potential to remove some of them in the future.
• Lastly, while I haven't done it, it might be nice to add a mypy run to tox.ini to ensure future commits/PRs don't break the type hinting (i.e. have the same as mypy --strict ./eel return successfully). These need only be run once with 3.10 - any backward incompatible typing annotations will raise runtime errors and already be caught during the existing pytest runs.

[samuelhwilliams]

Hi @thatfloflo - incredible work, thank you!

Here's a patch that I worked on last night actually, that should add the check to our CI. You might want to update it to include a --strict flag since you've worked on that. Could you add something derived from this please?

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 39f0785..0aeec38 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -25,4 +25,18 @@ jobs:
       - name: Setup test execution environment.
         run: pip3 install -r requirements-meta.txt
       - name: Run tox tests
-        run: tox -- --durations=0 --timeout=30
+        run: tox -e test -- --durations=0 --timeout=30
+  typecheck:
+    runs-on: ubuntu-20.04
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v2
+      - name: Setup python
+        uses: actions/setup-python@v2
+        with:
+          python-version: "3.10"
+      - name: Setup test execution environment.
+        run: pip3 install -r requirements-meta.txt
+      - name: Run tox tests
+        run: tox -e type
diff --git a/requirements-test.txt b/requirements-test.txt
index 0a9d972..90ec24d 100644
--- a/requirements-test.txt
+++ b/requirements-test.txt
@@ -4,4 +4,5 @@ psutil==5.9.2
 pytest==7.0.1
 pytest-timeout==2.1.0
 selenium==3.141.0
-webdriver_manager==3.7.1
\ No newline at end of file
+webdriver_manager==3.7.1
+mypy==0.971
diff --git a/tox.ini b/tox.ini
index fefe247..cf30775 100644
--- a/tox.ini
+++ b/tox.ini
@@ -1,5 +1,5 @@
 [tox]
-envlist = py36,py37,py38,py39,py310
+envlist = type,py{36,37,38,39,310}
 
 [pytest]
 timeout = 30
@@ -13,9 +13,16 @@ python =
     3.10: py310
 
 [testenv]
+description = run py.test tests
 deps = -r requirements-test.txt
 commands =
   # this ugly hack is here because:
   # https://github.com/tox-dev/tox/issues/149
   pip install -q -r '{toxinidir}'/requirements-test.txt
-  '{envpython}' -m pytest {posargs}
\ No newline at end of file
+  '{envpython}' -m pytest {posargs}
+
+[testenv:type]
+description = run type checks
+deps = -r requirements-test.txt
+commands =
+  mypy {posargs:eel}

[samuelhwilliams]

@thatfloflo I've just pushed a tiny commit to your branch to add some line endings back (github complains if they're missing). Otherwise I think this all looks good!

Would you mind squashing all of the commits down into a single one? I can do it my end but I'm not sure what that does to author attribution and want to make sure the commit that goes in is clearly your contribution.

[thatfloflo]

Thanks @samuelhwilliams, I've squashed this down to a single commit now (hopefully the right way, the squashed commit seems to include some intermediate merges from main - I would appreciate a pointer if I ought to fix that in some way).

[samuelhwilliams]

Ah, if we can rewrite the commit to just describe the changeset itself rather than reference the full log of commits (that won't end up in the main branch anyway) that would be good. Otherwise yes, looks broadly good 👍 Really useful change that should help people use Eel, I'm really grateful.

You made a lot of good points around refactoring. I think if we ever aim for a proper v1 release it will change the interface quite a bit, which would help clear up some of the typing issues we've got. For now what you've done is a fair compromise around the rough edges.

Might need to drop pyinstaller down to 4.10 based on the 3.6 errors, unless we needed 5.x for something in higher versions?

[thatfloflo]

I've tidied up the commit message.

I'm not quite sure what's happened with some of the failures on 3.7/3.9 now, I'll have to have a closer look at that and see how that can be addressed. 3.6 failure might be resolved by adjusting the PyInstaller version in requirements-test.txt, shouldn't really affect the ability of end-users to build with a newer version and the package doesn't have type information currently, so no real loss there I think. I can probably tinker a bit more later in the week to try and get this to work.

[thatfloflo]

So, it looks like the 3.7/3.9 failures on windows earlier were due to the chrome driver update, which I've also got a once-in-ten on my fork's run yesterday - it ran clean twice now so think we're good.

Otherwise, dropping down to PyInstaller 4.10 seems to have done the trick!

[samuelhwilliams]

Amazing. Let's get this in! 🎉 Thanks for all of your hard work, and apologies that this has taken almost a year to get in :(

[thatfloflo]

Brilliant, and no worries about the delay, in this case only meant an improved PR. Thanks for your feedback along the way :)

[ChrisKnott]

Yeah thanks @thatfloflo you rock!

(and @samuelhwilliams 😒)