Change JS config to use a schema and JSON #794
Conversation
|
I'm also updating the |
There was a problem hiding this comment.
Left one comment, I'm mostly trying to reconcile these changes with our conversation earlier today. This is a sketch of my understanding from our discussion:
- Vanilla plugin: lives in tree, and it's a generic JS environment. This is not our current
corecrate. - When specifying
-C dynamicand/or-C plugin,-Joptions are not allowed (this is the most simple route). We expect, at least initially or until someone actually needs this functionality that the plugin'sinitialize_runtimewill take care of initializing the JS runtime according to the plugin's needs. So no-Joptions are needed immediately. -Jcould be allowed for the vanilla plugin, which we don't have today and don't have to introduce immediately.- Ultimately our goal, at least in my head, is to remove the
corecrate entirely from Javy itself and treat it as plugin moving forward.
It's totally possible that I misunderstood some of our discussion points, so thanks for bearing with me.
In general, if my sketch above is not totally off, I think we can keep most of the changes here, except for the shared config parsing, which would involve eliminating parsing the -J group from the CLI.
| let mut config_bytes = vec![]; | ||
| let shared_config = match io::stdin().read_to_end(&mut config_bytes) { | ||
| Ok(0) => None, | ||
| Ok(_) => Some(SharedConfig::parse_from_json(&config_bytes).unwrap()), | ||
| Err(e) => panic!("Error reading from stdin: {e}"), | ||
| }; | ||
| let runtime = runtime::new(js_runtime_config).unwrap(); | ||
| if let Some(shared_config) = shared_config { | ||
| shared_config.apply_to_config(&mut config); | ||
| } |
There was a problem hiding this comment.
My comments are geared toward trying to reconcile the changes with our discussion earlier today; I think we agreed on going the simple route of not allowing any configuration to be passed for plugins from the CLI, except for the vanilla plugin. However, maybe we could've discussed what the vanilla plugin is; in my head it's not our current core crate; in my mind the ideal state would be to remove the core crate as we know it today from our tree and treat it as a plugin moving forward. I was thinking of vanilla as a generic JS environment, which we don't have today, and probably don't need to introduce immediately.
With that in mind:
- Can we instead remove the configuration overall? It's going to be a breaking change for sure, however some of the changes introduced for plugins already involve a breaking change.
- Aside from the breaking change aspect, I think the only thing that relies on the CLI flags affecting the runtime configuration are the integration tests.
Description of the change
This updates a few things relating to moving away from using a shared bitflag representation for JS configuration options between the Javy CLI,
javy-core, and the Javy library crate:shared_configCargo feature is added to the Javy library crate. Enabling this feature exports a Wasm function calledconfig_schemathat outputs a JSON representation of all of the configuration properties the provider supports. It also exports some Rust data structures and associated methods for use by thejavy-corecrate. These data structures and methods are used to parse a JSON data structure representing a JS configuration sent by the Javy CLI and apply the parsed representation to a pre-existingJavy::Configconfig.javy-corehas been updated to enable the newshared_configfeature and use its data structures to read in the JS configuration from the Javy CLI.config_schemaWasm function exported from the provider and to validate the-Jflags against this schema. It has also been updated to encode the JS configuration as a JSON byte array when passing the JS configuration to theinitialize_runtimefunction in the provider.javy-configcrate has been deleted which involved updating the Makefile and GitHub Action workflows.This change would not support runtime configurations that are not defined in the Javy crate. This change also only supports boolean configurations. It should be possible to augment the approach we're using if there is a need to in the future.
Why am I making this change?
Part of #768. Since plugins will exist outside of the Javy source tree, they cannot depend on the unpublished
javy-configcrate. Also it's easier for bitflags to get out of sync betweeen different source trees and when the bitflags get out of sync, the error messages that result are not helpful. So, we need a way for the Javy CLI to know what configuration properties a plugin supports along with associated help text. We also need a way for the CLI and plugin to understand each other when a configuration is passed intoinitialize_runtime. JSON with using UTF-8 strings to describe the configuration properties seems like a relatively straightforward approach.Checklist
javy-cliandjavy-coredo not require updating CHANGELOG files.