tink: Webpackify

help-wanted
tink

(Kat Marchán) #1

I’d really like tink to be released bundled and minified, preferably by Webpack. Having someone take the time to get tink compiling, and making sure its worker still works and making it so the final bundle is as small as possible would be tremendously helpful.

Do you know webpack? Are you interested in helping out with this? Reply below to claim this task! I’ll be available to answer any and all questions for any kind soul considerate enough to do this for me. :sparkles:.


tink: State of the Unwinder [2018/11/05]
(Jae Anne Bach Hardie) #2

I’m willing to help with this. Done quite a bit of webpacking here and there. Do you want all its dependencies to be bundled with it? I assume that’s part of the objective, so the end user doesn’t need to use npm to install tink.


(Kat Marchán) #3

Yes, I’d like all dependencies bundled (and compressed).


(Jae Anne Bach Hardie) #4

Cool. Might have to get creative if any of the dependencies have native addons that need compiled for all target architectures. I’ll take a look over the weekend if that’s OK.


(Kat Marchán) #5

None of them have native deps – the only two complications I can think of right now are:

  1. There’s a worker process that needs to be a separate file and still be loadable by lib/commands/shell.js
  2. yargs has given me trouble with webpack before. If it doesn’t want to compile for you, just exclude it from the bundle and do "bundleDependencies": ["yargs"] in package.json instead (which is how it currently is, 'cause I was starting to webpack tink)

And yes, taking a look at it this weekend would be fantastic. If it ends up feeling like too much, just let me know and we can see if someone else picks this up :slight_smile:


(Jae Anne Bach Hardie) #6

Hey, I’m a bit confused by ping.js:10, which goes
options: Object.assign(require('../common-opts.js', {})),

What is that two-argument require doing? I’ve not seen it before and webpack doesn’t know what to do with it.


(Kat Marchán) #7

That’s a bug! It should be one argument with {} as a placeholder second argument to Object.assign


(Jae Anne Bach Hardie) #8

Ah, cool. I’ve got to a point where it’s building with a few caveats (spawn-wrap and libnpm do things which complicated bundling so I’ve left them as externals for now). I want to test all of the functionality out to make sure it still works and then I’ll try to work around the issues with those two dependencies.

There’s also a lot of duplicated code between the initial bundle and the worker bundle right now because code splitting entry chunks in NodeJS seems to be something nobody had considered doing before now so that’s also something to tackle in the future.


(Kat Marchán) #9

The code’s also changing a lot right now so it might be best to leave it at the current baseline and wait until things stabilize again (I expect the current massive refactor to be done by end of the week – I’m working on it as we speak, and it involves notable changes to which workers are needed. It’s so awesome that you’ve got something building so far!

What’s stopping libnpm from being bundled? It should be basically a straight-up conduit package. If there’s anything I can do to fix that, let me know and we can fix that right up.

Thanks again. I’ll keep you up to date as the refactor happens (tl;dr I’m getting rid of .package-map.json and changing the way things get installed and loaded, and I’ve completely overhauled the way commands get loaded, with webpack in mind a bit more).


(Jae Anne Bach Hardie) #10

libnpm calls npm-lifecycle which calls node-gyp in some odd ways. I didn’t get to the bottom of it because I figured I’d better get something building first.

Suits me fine to wait for a week or so tbh since I’m traveling for work next week and wasn’t going to have much time to set aside anyway.


(Kat Marchán) #11

ah, yes. npm-lifecycle expects a node-gyp binary to exist somewhere. What you should externalize is node-gyp itself, and I thought the rest was invoked as a child process.


(Jae Anne Bach Hardie) #12

Yeah. Hopefully I can do some clever transformations on spawn-sync to execute its fs.readSync call on bundling and maybe have a poke at yargs as well so the only bundled dependency we’d need is the node-gyp binary. There’s nothing to be done about binaries but everything else is in theory tractable.


(Daniel Stockman) #13

Webpacking CommonJS modules also runs afoul of intentional circularity, in my experience. Specifically, readable-stream needs to be omitted from the bundle (external, if present) because poor lil rollup-plugin-commonjs really has no way of accurately transforming it.

Excited to see progress!


(Kat Marchán) #14

Hey y’all!

I’m pretty sure we’re good to start this effort again!

At this point, I would like a separate entry point for each command so we can reduce parse time, and there’s only one worker that needs to be handled separately. Thanks again for your help with this!


(Jae Anne Bach Hardie) #15

OK, taking some time to look at this again.

So, you want one bundle per command and the runner to only require the one matching the first argument? That’ll require separating the command config from the command execution code so yargs can load its config without loading all the commands.


(Kat Marchán) #16

Indeed. Would you like me to do that refactor for you or would you like to take it on?


(Kat Marchán) #17

update: I’m doing this refactor for you! I’ll have it ready in a jiffy.


(Kat Marchán) #18

@jbachhardie I’ve completed the refactor and made things a bit lazy to increase startup speeds some.

tl;dr: The specs for the various commands have to go into the main bundle, and that includes everything in lib/yargs-modules. Each command is then isolated and lazily loaded from lib/commands and each one in that directory can be split into its own entry point.

hope this helps. Let me know if there’s anything else I can do to unblock you, and thanks again for the help!