RFC: Should the NPM package @nativephp/electron-plugin be a part of the nativephp/electron Composer package?

[simonhamp]

Rationale

Right now, the separation of these two packages causes some overhead:

1. A new release of the NPM package (electron-plugin) requires an 'unusual' workflow (npm version ... && npm publish); This is different from the more convenient approach that Composer/Packagist use: watching and referencing GitHub releases.
2. A new release of electron-plugin forces a release of nativephp/electron, to update the version of electron-plugin that it references internally (resources/js/package.json).
3. Testing during development is slightly harder due to the doubly-nested dependency, and this always forces the developer to edit 'core' files (mainly the package.json) in order to use their local code instead of the pre-built dist code from npm.

Proposal

Move electron-plugin code into resources/js/electron-plugin in nativephp/electron.

As nativephp/electron is the only part of the toolchain that references electron-plugin, it could easily reference a file path internal to itself and doesn't need to be listed at all on NPM (or other Node package directory).

Result

1. electron-plugin would cease to exist as a separate dependency and repository - its files would be included directly in nativephp/electron and then referenced by a relative path from the internal package.json.
2. There would be no need for an unusual release process via NPM; a single release of nativephp/electron would be all that's required.
3. Instead, with the Node dependency 'baked in', the dependency manager (npm etc) could simply build and pull in the local code, rather than being dependent upon another third-party service to fetch a pre-built version.
4. Testing of WIP changes would not require temporary modification of the package.json and could 'just work' as part of the native:serve command.

---

Perhaps I haven't thought of everything here. Maybe there's some problem with this approach that I'm being completely blind to? Please let me know your thoughts in the comments.

[simonhamp]

One snag with attempting this is that npm by default only uses symlinks when importing a local (filesystem-based) package.

We can't have symlinks because the app can't be bundled with a symlink - we need hard copies.

There's a workaround, but it requires us to do this 'packing' step and will still require some kind of update mechanism to run during development.

But it does solve the problem of colocation...

[gwleuverink]

Interesting. Would watch-mode when working on the electron-plugin still work with this extra packing step?

Maybe the --install-links flag could also provide a solution?

The docs mention it doesn't symlink but rather pack & install in one go. I haven't used it before but can experiment with it if you haven't already.

The downside (after a quick google) is that if you manually run npm install or npm update it will break the link. Not sure if that's an issue since we can make sure it is linked correctly from the serve command.

[simonhamp]

Interesting! I'll give it a go

[gwleuverink]

Spent a couple hours yesterday evening and ran into some issues when running build:watch in the plugin dir then running native:serve. Something about types being defined again.

That was fixed by replacing npm install with npm ci in the serve command. But I'm not sure what caused the issue exactly so this needs to be checked.

The same problem came back when running native:build. It perhaps has something todo with the fact that the build command runs npm update under the hood.

My next step was to try and add a npmrc file and play with the install-links option.

Here's what I got so far (without that npm ci change)
https://github.com/gwleuverink/nativephp-electron