Plugin system

[6utt3rfly]

Staging area before releasing all plugins to main

[6utt3rfly]

@LeaVerou - I merged the approved PRs into this branch because I was thinking it would be better to ensure everything is working together and that we're happy with the API, documentation, etc. before merging all of it into master and releasing a new version. Buf if you would prefer to merge straight into main that's fine with me too 🤷🏻‍♀️

[LeaVerou]

I'm not used to the workflow of releasing a new version every time a PR is merged, but I'm not opposed to it either. 🤷🏼‍♀️
Ok, let's get consensus on the existing plugin PRs before merging. What's left to review?

[6utt3rfly]

not used to the workflow of releasing a new version every time a PR is merged

I typically don't either ... but I thought for a significant change like this it might make more sense. Especially if we're updating the readme docs but haven't yet published to npm 🤷🏻‍♀️

At any rate, I think it's nearly ready. I just updated the .npmignore file after I noticed what was included (npm pack builds a tar file to test).

One issue I noticed, when trying to use the build in a typescript project, is that the plugins themselves don't have a type definition:

import * as jsep from 'jsep';
import * as jsepArrow from 'jsep/dist/cjs/jsepArrow.cjs.js';

TS7016: Could not find a declaration file for module
'jsep/dist/cjs/jsepArrow.cjs.js'. 'node_modules/jsep/dist/cjs/jsepArrow.cjs.js' implicitly has an 'any' type.   Try `npm i --save-dev @types/jsep` if it exists or add a new declaration (.d.ts) file containing `declare module 'jsep/dist/cjs/jsepArrow.cjs.js';`

I could fix it by adding types to the tsd.ts file:

declare module 'jsep/dist/jsepArrow' {
  import * as jsep from 'jsep';
  const name: string;
  const init: (this: typeof jsep) => void;
}

declare module 'jsep/dist/cjs/jsepArrow.cjs.js' {
  import * as jsep from 'jsep';
  const name: string;
  const init: (this: typeof jsep) => void;
}

declare module 'jsep/dist/jsepArrow' {
  import * as jsep from 'jsep';
  const name: string;
  const init: (this: typeof jsep) => void;
}

declare module 'jsep/dist/iife/jsepArrow.iife.js' {
  import * as jsep from 'jsep';
  const name: string;
  const init: (this: typeof jsep) => void;
}

Or do you have a better suggestion? (I'd hate to do nothing and force 'allow implicit any', or have to manually declare the module)

[LeaVerou]

Can we have separate typings per module, included in its own directory? I'd like them to be as self-contained as possible.

[6utt3rfly]

Maybe we should make the plugins more like a mono repo then? So that each plugin is individually published to npm (i.e. @jsep/arrow-plugin)? Might be nicer for versioning in the future if each has their own version? See #158

[6utt3rfly]

@LeaVerou and @EricSmekens - I think this should have all of the PRs and changes that were already individually reviewed and approved. Do you want one more review of the branch before merging all to main? With the separate packages per plugin, should the github actions be updated?

[EricSmekens]

Approval on my side on merging this all to main.

Thank you for all your time and work on this! I think the actions should be updated, but I'm fine with that in a seperate pr.
(It currently uses npm publish, I guess for the multiple packages we need something different).

[6utt3rfly]

I guess for the multiple packages we need something different).

I think each plugin package could be manually published (cd packages/arrow && npm publish) in the meantime? :-)

[LeaVerou]

Approval on my side too. We can always iterate with other PRs.

[6utt3rfly]

FYI - I started working on updating expression-eval to support the new node types and have been confirming that it can evaluate the nodes and use the updated jsep and packages. Merging! :-)

[dereklieu]

This is exciting, I'm looking forward to using the object plugin. Do you have plans to include this work in an upcoming release?