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

Add a javascript runner for running javascript before and after requests #516

Closed
wants to merge 1 commit into from

Conversation

mrloop
Copy link

@mrloop mrloop commented Jan 22, 2025

This implement a basic javascript runner.

  • support Response API
  • support set/get Request API variables
  • support set/get client global variables
  • support jsonPath query in javascript

Dev notes:
A intermediate file is created for each request in the script folder, last_javascript.mjs is the last javascript file that was run. This is useful for debugging. For instance you can run the file from the command line node --inspect-brk last_javascript.mjs and then open up chrome://inspect in chrome based browers and debug the javascript.

@mrloop mrloop force-pushed the javascript_runner branch 3 times, most recently from b86f3bb to 17a25cb Compare January 23, 2025 08:43
Copy link
Contributor

@boltlessengineer boltlessengineer left a comment

Choose a reason for hiding this comment

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

Thank you for your interest and all works!
Didn't expect someone will contribute to javascript runner.

Though I think injecting all data directly to javascript code is too hacky. Both environment variables and response data can be quite large. I'm thinking of communicating with node client using jsonrpc to share data asynchronously.

local script = {}
local logger = require("rest-nvim.logger")

local has_js = vim.fn.has('node')
Copy link
Contributor

Choose a reason for hiding this comment

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

has() is for checking features, not executables. It will check if Neovim node provider is loaded (:h node-client, :h g:loaded_node_provider) and running node is still possible when node provider is not loaded.

We can use vim.fn.executable('node') instead. (:h executable())

Copy link
Author

Choose a reason for hiding this comment

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

updated

end

local function write_file(filename, content)
local file_to_open = cwd() .. "/" .. filename
Copy link
Contributor

Choose a reason for hiding this comment

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

if script file is saved for temporary use, it should be saved in temporary directory like $XDG_RUNTIME_DIR where you can find with stdpath("run")

Copy link
Author

Choose a reason for hiding this comment

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

not sure the best way to do this and still let node resolve dependencies

local function create_handler_env(ctx, res)
local env_vars = {}
-- can't do pairs(vim.env) instead need to use environ
for k, v in pairs(vim.fn.environ()) do
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if passing environment variables is necessary. vim.env table will be available as normal environment variables in processes under Neovim instance.

Copy link
Author

Choose a reason for hiding this comment

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

Using process.env in node to read environment variables now

-- can't do pairs(vim.env) instead need to use environ
for k, v in pairs(vim.fn.environ()) do
-- ignore values which will make escaping complicated
if not string.find(v, "[\\'\"`]") then
Copy link
Contributor

Choose a reason for hiding this comment

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

just not handling variables with quotes feel too hacky imho.

Copy link
Author

Choose a reason for hiding this comment

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

no longer copying all the things

This implement a basic javascript runner.
* support Response API
* support set/get Request API variables
* support set/get client global variables
* support jsonPath query in javascript

Dev notes:
A intermediate file is created for each request in the script folder,
`last_javascript.mjs` is the last javascript file that was run.
This is useful for debugging. For instance you can run the file from the
command line `node --inspect-brk last_javascript.mjs` and then open up
`chrome://inspect` in chrome based browers and debug the javascript.
@mrloop mrloop force-pushed the javascript_runner branch from 17a25cb to 335f997 Compare January 26, 2025 18:27
@mrloop
Copy link
Author

mrloop commented Jan 26, 2025

Thanks for reviewing

I'm thinking of communicating with node client using jsonrpc to share data asynchronously.

That sounds good. Would you still need something like javascript.mjs and then the jsonrpc would be used for setting the global and request variables only? Is this PR good enough in the mean time?

@boltlessengineer
Copy link
Contributor

I think it would be better to prioritize implementing a proper jsonrpc version from the start. This approach will take more time, but it will provide a more maintainable and appropriate solution in the long run.

For this reason, I’m closing this PR, but I truly value your work and hope you’ll continue contributing in the future. :)

@mrloop
Copy link
Author

mrloop commented Jan 28, 2025

I look forward to seeing the jsonrpc implementation

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