Skip to content

Conversation

Hamada-Distributed
Copy link
Contributor

Adds some pythonic file handling and some pythonic type hinting, as well as modifies the python load function to use the importlib machinery so that python modules are loaded conforming to python spec.

  • Pythonic file handling: Python's open doesn't automatically close the file handler, it's recommended to use with open() as f: to ensure the file handler closes.
  • Pythonic type hinting: Does absolutely nothing to the actual runtime, but allows for devs to have some insight into types using IDEs and the such
  • python load function using importlib machinery: Allows for the use of the importlib machinery sourcefilereader loader to do the heavy lifting of importing the python module. Since this loader memoizes the module in sys.modules, we also check to make sure that sys.modules doesn't already have the module loaded in.

def helloWorld():
print('hello, world!')

exports = {}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modules should not provide their own exports object; the module system should provide this. CommonJS modules are decorator-pattern, with the exports object memoized before the module is evaluated. That is key to, among other things, correct behaviour when loading (potentially transitive) circular dependencies.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that makes sense. I'll modify the python load to prefill the module with the exports object so that when the loader loads the module the exports object should exist and be memorized before the module is evaluated.

@wesgarland wesgarland merged commit 27c8ee0 into wes/module-system Jun 22, 2023
@wesgarland
Copy link
Collaborator

I've merged this into wes/module-system

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants