Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Module manager POC #524

Open
wants to merge 27 commits into
base: master
Choose a base branch
from
Open

Conversation

KaruroChori
Copy link
Contributor

@KaruroChori KaruroChori commented May 19, 2024

Ref: #514

Proof of concept of the external module system. It works, but more testing is needed.
I prepared a demo module @ https://github.com/KaruroChori/txiki-module-demo.
I also have a more realistic example with a port of the popular commander @ https://github.com/KaruroChori/commander.js

To test this feature, add a modules.json file in the root of the project (it will not be tracked) or anywhere else. If not in the default location you will have to pass the filename as argument to the CLI command.
Inside the json file is something like this:

{
    "trialmode": "https://github.com/KaruroChori/txiki-module-demo/archive/refs/tags/v2.0.0.tar.gz"
}

Modules can either be tar.gz files distributed online, or local folder.

After that, you will find there two new npm scripts available, or just extras-helper.mjs to be used via command line.

Features:

  • Allow extra dependencies
  • fetch/untar (*via shell)
  • Tests unpacking
  • Examples unpacking
  • Benchmarks unpacking
  • Typescript types unpacking, docs can be generated as well
  • JS code unpacking
  • C code unpacking

I also took the liberty to fix few more issues which can be separately handled and ported over to the master branch:

  • The lack of prefix / in most of the .gitignore entries.
  • Some duplication of rules in the CMake file.
  • A quite old fashioned mechanism to import types in the d.ts files.

@KaruroChori
Copy link
Contributor Author

KaruroChori commented May 19, 2024

The current implementation is using shell commands to untargz the module if downloaded.
Considering windows is also a target, this is not ideal. A library should be picked to do this job in a more portable way.

I am stopping here for the time being, since all the basic functionality I wanted is there, and I need some feedback.

@KaruroChori KaruroChori marked this pull request as ready for review May 22, 2024 12:08
@KaruroChori
Copy link
Contributor Author

KaruroChori commented May 22, 2024

@saghul I implemented all features I needed as originally discussed in #514.
Allowing extra dependencies is something which could be easily done, but since I have no use for that at this stage, I left it out. The initial comment has been revised with additional information.

I published two repos to demo how modules should be structured, one with the basic boilerplate, and the other of a real library being wrapped (commander.js now fully async-ified to properly use txiki core modules)

CMakeLists.txt Outdated Show resolved Hide resolved
@saghul
Copy link
Owner

saghul commented May 28, 2024

I've a bit busy these days, I hope to take a close look at this next week!

@KaruroChori
Copy link
Contributor Author

I finally added support to include external native deps as part of the build step of a custom runtime.
For example, a lvgl-js module can now ask for lvgl & lvgl_drivers to be cloned alongside the other deps.

As I received no feedback so far, I just implemented everything according to my needs on a separate branch, and back-ported it here. I am in the process of documenting how to use the features suggested in this PR on this separate repo.

Aside from bugfixing, I have no plan for more features.

@KaruroChori
Copy link
Contributor Author

KaruroChori commented May 28, 2024

I've a bit busy these days, I hope to take a close look at this next week!

No problem. Please, let me know if you encounter any issue, as the documentation for this is still spotty at best & the implementation was a moving target up until now.

karurochari and others added 3 commits June 7, 2024 16:01
Fixes issues in the documentation generation with modern syntax.
Aligned with the latest protocol 2.2.0
@KaruroChori
Copy link
Contributor Author

KaruroChori commented Jul 2, 2024

@saghul, this PR has not been evaluated, or at least it has received no feedback so far.
I am now planning a further expansion, but forcing compatibility with this branch when it is not even clear if the module feature has a path for integration would make no sense; so I am basing it directly on my divergent fork.
Basically I am shifting the tjs interface around (#539) to be less redundant and crowded, moving to a semantic versioning scheme, and few other changes on top of what has diverged already.

As a side effect, it means that in few weeks all node packets I ported over or natively wrote for txiki to be used as either npm dependencies or external modules will have to be updated and they are no longer going to work with this master branch.
If there is interest, the best I can do is to branch them off as they are now for future reference or usage.

@saghul
Copy link
Owner

saghul commented Jul 2, 2024

Now that I got some of the bigger things out of the way I hope to take a look at this PR this week.

Basically I am shifting the tjs interface around (#539) to be less redundant and crowded, moving to a semantic versioning scheme, and few other changes on top of what has diverged already.

I think it's better to have that discussion before diverging (but feel free to do whatever you want with your project, of course!) because I'm not sure I'll go for it.

@KaruroChori
Copy link
Contributor Author

No worries, I am not really trying to push anyone into doing anything or to affect your vision behind this project :D.

But my projects have needs on their own; so, my general attitude is contribute to master what is feasible in time and aligned in scope to the main branch.
That being said some degree of forking is mostly inevitable.

Still, I am more than open to discuss them early to keep the amount of divergence to a minimum.
As for the interface, I think there are some objective flaws. They all have limited severity, but the compounded effect makes the experience a bit frustrating:

  • Redundancy of some elements
  • Sometimes objects are exposed, sometimes functions returning objects.
  • Inconsistent casing
  • Limited documentation of most entries
  • Lack of versioning guidelines to ensure new releases are compatible with older code (which goes back to the issue I have with versioning)

But this discussion should probably continue in #539 or whatever evolved proposal derived from it.

@saghul
Copy link
Owner

saghul commented Jul 12, 2024

Hey @KaruroChori sorry for the delay. I've been giving this a thought this week. It's a lot of code to bring in on one go, so let's break it down.

Your approach consists of 3 parts:

  1. The ability to add native code to the build
  2. The ability to add JS code to the build
  3. Some scripting to make 1 and 2 easier

I wonder what other options we have, so we can get closer, in a way that doesn't seem to be that involved.

For starters, 3) could live out of the project at least while we sort things out.

For adding native code to the build there are a few options:

  • Adding support for shared library modules: this request comes and goes, and I wanted FFI to be the answer to that... but it wouldn't hurt to add.
  • Since you can now build txiki.js as a library, add the missing APIs so we turn this around and make a new application which embdeds txiki.js, initializes the engine, registers some extra modules and starts the interpreter.

For adding JS code:

  • The C API could allow registering external JS modules the same way it already does with the stdlib.
  • Using the ability to create a standalone executable, bundle the code that way.

WDYT? Would any of these help you accomplish your goals?

@KaruroChori
Copy link
Contributor Author

Hopefully I will be able to properly reply over the weekend. I have to catch up with the latest changes in this repo first as you have been very active :D.

@KaruroChori
Copy link
Contributor Author

KaruroChori commented Aug 1, 2024

Your approach consists of 3 parts ...

As far as this repository would be concerned yes.
The specifications for the module.json file and how to structure a module are not relevant: point 3 serves as glue to keep txiki.js agnostic in this respect.

For starters, 3) could live out of the project at least while we sort things out.

While possible, it is just a single file implementing a CLI which must be manually invoked to do anything.
But I have nothing against addressing the rest of the proposal first.
As you suggested, that script is mostly arbitrary and it implements external specifications, so it can live outside.

Adding support for shared library modules: this request comes and goes, and I wanted FFI to be the answer to that... but it wouldn't hurt to add.
Since you can now build txiki.js as a library, add the missing APIs so we turn this around and make a new application which embdeds txiki.js, initializes the engine, registers some extra modules and starts the interpreter.

Both these options are valid but different from what I am trying to achieve.
With FFI one has no access to the low level quickjs and txiki specific code, as it is not being exposed.
So it would not be ok to extend the runtime, only to expose self-contained & high performance modules.

Using txiki as a library in this sense is more flexible, and might work in several cases. However, there is additional complexity to embed the project, and I would personally find it a bit of an overkill to just add some javascript modules as part of the runtime, even more so if I am not authoring them.

The solution I proposed only requires minimal changes to the current code to add some injection points for custom files. This allows to integrate our arbitrary C/C++ and JS code from other sources and keep it outside the git branch.

The C API could allow registering external JS modules the same way it already does with the stdlib.

This does not really solve the issue for hybrid modules.

Using the ability to create a standalone executable, bundle the code that way.

This does also not solve the issue of hybrid modules on its own, and it requires each executable to bundle any part that we want extra in the runtime.
Yet, the main reason to add features to the runtime is so that scripts and executables can be smaller when distributed. The cache layer I discussed in an other proposal might offer a hybrid approach in this respect, but unlike external modules it has not been implemented.

@KaruroChori
Copy link
Contributor Author

KaruroChori commented Aug 1, 2024

External references to this features and its implementation beyond the scope of txiki's codebase:
https://github.com/KaruroChori/txiki-modules
https://github.com/KaruroChori/demo-txiki-module

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.

3 participants