Skip to content

Fix JS runtime option help#1158

Open
jeffcharles wants to merge 2 commits intomainfrom
jc.fix-how-help-for-runtime-options-works
Open

Fix JS runtime option help#1158
jeffcharles wants to merge 2 commits intomainfrom
jc.fix-how-help-for-runtime-options-works

Conversation

@jeffcharles
Copy link
Copy Markdown
Collaborator

Description of the change

Move handling -J help into the options parser instead of performing it later. This means we can inline JsGroupOption wherever JsGroupValue was used since we're not propagating that help was requested. Also update the error message emitted when someone tries to use -J with a plugin to mention it's not supported when specifying a plugin.

Why am I making this change?

Fixes #1157.

I opted to move the handling with -J help into the parser because it seems like making input optional in this one particular case would introduce more complexity compared to just loading the default plugin to provide the runtime options. I updated the error message for using -J with plugins because it could be confusing for a user if the -J help message says a particular option is supported and then they get an error message saying that option is not supported when trying to use it with a plugin.

Checklist

  • I've updated the default plugin import namespace and incremented the major version of javy-plugin-api if the QuickJS bytecode has changed.
  • I've updated the relevant CHANGELOG files if necessary. Changes to javy-cli, javy-plugin, and javy-plugin-processing do not require updating CHANGELOG files.
  • I've updated the relevant crate versions if necessary. Versioning policy for library crates
  • I've updated documentation including crate documentation if necessary.

Copy link
Copy Markdown
Member

@saulecabrera saulecabrera left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this! Left one comment around test coverage.

}

#[test]
fn test_js_help() -> Result<()> {
Copy link
Copy Markdown
Member

@saulecabrera saulecabrera Mar 30, 2026

Choose a reason for hiding this comment

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

Skimming this file it seems that we don't have tests for build -C help? Could you also add a test for that as well?

Additionally, could we add an integration test that exercises that error cases (i.e., plugins)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'll add one for build -C help but test_using_wasip1_plugin_with_static_build_fails_with_runtime_config and test_using_wasip2_plugin_with_static_build_fails_with_runtime_config exercise the error case where -J options are provided and a plugin is specified and ensures they fail with a helpful error message. Calling -J help while providing a plugin does succeed because as I mentioned in the PR description, there isn't a good way to have that error without adding a bunch of complexity around the input parameter needing to become optional to support that.

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.

javy build -J help errors, does not list "intrinsic options"

2 participants