Reduce complexity of the plugin API about node-client HOT 26 CLOSED

I agree, I'll explore a simpler API for creating plugins

from node-client.

I don't know if any work has been done on this, but I'll offer another suggestion! My preference would be strongly against any global object being either injected or required.

Defining a plugin by exporting a single function which takes nvim as an argument would be ideal for testing via dependency injection etc. The functions to register commands could exist on the nvim object.

This would allow whatever programming style you prefer, functional or OO, eg:

function someCommand() {}
function someAutocmd() {}

class StatefulThing {
  aMethod() {}
}

class AnotherClass {
  constructor(nvim) {
    nvim.registerCommand('AnotherCommand', [this, this.anotherMethod]);
  }
  anotherMethod() {}
}

module.exports = (nvim) => {
  nvim.registerCommand('CommandName', someCommand, { option: true });
  nvim.registerAutocmd('BufWritePre', someAutocmd, { pattern: '*' });

  const stateful = new StatefulThing();
  nvim.registerCommand('AnotherCommand', [stateful, stateful.aMethod]);

  const anotherInstance = new AnotherClass(nvim);
};

I would also love an official way to listen for arbitrary events sent by rpcnotify. I'm currently hacking around this with the following:

@Plugin({ dev: true })
export default class ListenerPlugin {
  @Command('AddListener')
  async addListener() {
    this.nvim.command(`au CursorMoved * call rpcnotify(${this.nvim._channel_id}, '${PLUGIN_PATH}:command:Handler', 'Test')`);
  }

  @Command('Handler')
  async handler(...args) {
    console.log(args);
  }
}

It would be nice to have nvim.registerRPCHandler('EventName', fn) instead.

I'm happy to put in a PR for a different plugin API if it won't be duplicating work.

from node-client.

Now that #54 is merged, how does this affect local testing such as https://github.com/neovim/neovim/blob/master/test/functional/provider/nodejs_spec.lua? I worked on this with @justinmk but I'm not satisfied with the global dependency of the neovim module because it leaks the abstraction of the nodejs host into the plugin. Is require('neovim') still required in the user plugin?

from node-client.

Thanks for your answer. I'm looking forward to.

from node-client.

Doesn't work in Windows. I've tried on node 6.9.1 and current LTS (8.9.1).

from node-client.

@janlazo I wish I could help but I don't use and know Windows, but it should work; I don't think the problem this about the code sample. For what it's worth, that's not this issue for. I think you should create an another issue for that, with your exact steps.

from node-client.

This works for me in Windows. I can't register commands yet.

In Neovim before sourcing the nodejs rplugin

" required for globally-installed modules
let $NODE_PATH = split(system('npm root -g'), "\n")[0]
let g:nodejs_plugin_id = jobstart(['node', 'path/to/plugin.js'])

Code for nodejs plugin

const socket = process.env.NVIM_LISTEN_ADDRESS;
const neovim = require('neovim');
const nvim = neovim.attach({socket: socket});

class TestPlugin {
    echoHello() {
        this.nvim.command('echomsg "hello"')
    }
}

const PluginClass = neovim.Plugin(TestPlugin);
const plugin = new PluginClass(nvim);
plugin.echoHello();

from node-client.

Is there a way to avoid users/plugin authors needing to manually manage the $NODE_PATH ?

In the python-client we mangle the import path dynamically. Although virtualenv's must be managed by the user (obviously).

from node-client.

@janlazo You shouldn't need to do jobstart(['node', 'path/to/plugin.js']), that's what the node host is supposed to handle.

from node-client.

@billyvg What about plugins outside of rplugin/node?

from node-client.

What's the use case where it's outside of rplugin?

from node-client.

How do you feel about something like this, (similar to old API, except we have to import neovim/plugin)

// I think let's inject this as a global
// const Plugin = require('neovim/plugin');

const plugin = Plugin.create('PluginName');

plugin.function('FuncName', {options}, function() {
});

plugin.command('commandName', {options}, function() {
});

plugin.autocmd('BufEnter', {pattern: '*.js', eval: 'expand("<afile>")', sync: true }, function() {
});

// Probably unnecessary
module.exports = plugin;

from node-client.

@billyvg What do you think about this one? Just sharing it for the sake of more ideas.

const { Plugin } = require('neovim')

class TestPlugin extends Plugin {
	constructor() {
		super()

		this.register({
			as: 'plugin',
			name: 'TestPlugin'
		})
	}

	testFunc() {
		this.register({
			as: 'function',
			name: 'FuncName'
		})

		// ...
	}
}

from node-client.

My experience with javascript is mostly on ES5 in the browser so I'm fine with a few global function/class/object to interact with the client to avoid global require as long as I don't have to use an IIFE. I prefer to not use a class because Vimscript doesn't have classes but if that's what it takes to hide nvim or avoid IIFE, I can live with it.

If we're using classes, I prefer to have a custom class constructor on top of base plugin class for more control on the initial state.

from node-client.

If we're using classes, I prefer to have a custom class constructor on top of base plugin class for more control on the initial state.

@janlazo Like my example?

from node-client.

Yes, but I'm not sure about this.register because it seems too generic to expose. I prefer passing an object to super() to hide this. Worst case, the user can use this.nvim.command to define commands, functions, etc. after creating a new instance of the user plugin. Maybe there's a way to broadcast that event or perhaps add a promise handler.

from node-client.

@justinmk npm link neovim reduces the need to set NODE_PATH but that creates a local node_modules folder and copies a subset of the required files (and all of its dependencies) on Windows instead of creating a symlink. ln.exe -s behaves the same (junegunn/fzf#933 (comment)). It's best to stick with NODE_PATH if the user doesn't require package.json for dependencies.

Reference: https://docs.npmjs.com/cli/link

from node-client.

Sounds good. In any case this is done by node-client, right? Plugin devs and users don't need to do this explicitly.

from node-client.

Out of curiosity, why not let developers worry about the implementation and simply provide them an object from which to use?

export default function myPlugin(nvim) {
  function someCommandA(register) {
    register.command('SomeCommandA');
    return (args) => nvim.command('someCommandA');
  }

  function someCommandB(register) {
    register.command('SomeCommandB');
    return (args) => nvim.command('someCommandB');
  }
  
  return {
    someCommandA,
    someCommandB,
  };
}

Allowing developers to leverage the power of closures instead of forcing them to use ES6 classes would be preferable, imo.

from node-client.

@simlrh AFAIK we have to require/import the neovim module but I generally love your idea.

from node-client.

I'm ok with current API, but the code transform is required at this time, it makes it harder to write and debug, a functional API would be nice to have.

from node-client.

@chemzqm As I mentioned in my first code example in this issue, you don't have to transform your code with Babel, but if you don't you have to write your decorators like Command('Vsplit')(TestPlugin.prototype.splitV). Are you still okay with that? What approach do you prefer instead of decorators?

from node-client.

@kutsan I would use babel instead of write code like Command('Vsplit')(TestPlugin.prototype.splitV) it's not my taste. I've been using mobxjs/mobx for a long time, so I'm comfortable with the decorators.
Maybe node-client could provide a command that wraps babel to use transform-es2015-modules-commonjs transform-decorators-legacy for code transform, so that the plugin author could avoid install them for every plugin.

from node-client.

@chemzqm I don't think providing a wrapper around babel is a good idea.

from node-client.

Since @simlrh did a great job by making #54 I think we can close this issue now. 🎉

from node-client.

Is it ready for public testing? There's no patch release. I assume require('neovim') is still required for the plugin (not host) so I don't see a reason to justify updating the neovim tests yet.

from node-client.