Add initial prototype

[tniessen]

I finally had time to restructure and rewrite at least a part of my prototype. It is absolutely rudimentary, but I think it can still serve as a basic structure.

For reference, @sam-github, @mhdawson and I discussed possible WebCrypto implementations at the summit in Berlin. We decided to explore possibilities within external, non-native packages first and to discuss a built-in or native module at a later point. See https://github.com/nodejs/admin/issues/366.

(Sorry Sam, it took slightly longer than I claimed it would.)

[rvagg]

Wow, that's a lot of work in there @tniessen! good job. That manual AES stuff is a bit annoying, maybe we can push that functionality into core though, wrap it around our OpenSSL calls. For now just make it work is a good rule to follow.

Re testing and "It is our intention to add Web Platform Tests (WPT) at some point", what is that point and why not start doing it now? Is there no way to start importing portions of the WebCrypto conformance tests to guide development?

A couple of stylistic concerns:

1. Is Babel really necessary? Dependency bloat where it's unnecessary and the layers of complexity it adds contributes to contribution barriers. From what I can see the only thing it's contributing here is translating import to require, can you justify its inclusion? If you have target versions of Node in mind you can just use linting rules to keep conformance.
2. More comprehensive linting rules up front will save a lot of pain and argument later on. I'd suggest just adopting standard (or semi-standard if you insist), you can just pull the eslint rules as separate packages for those if you'd prefer. Or, pull the nodejs/node rules if you'd prefer them. Whatever it is, setting tight boundaries now will make reviewing code later easier.

[gengjiawen]

I am -1 on Babel if we target it to Node:12.

[tniessen]

@rvagg Thank you for the feedback!

That manual AES stuff is a bit annoying, maybe we can push that functionality into core though, wrap it around our OpenSSL calls.

We could, even though the implementation probably wouldn't be much prettier since OpenSSL does not support it natively, and I don't think we should try to move it into node core or OpenSSL until someone comes up with a reasonable use case. I thought about this quite a lot while implementing it and I still don't know why any length != 128 would be advisable.

Is there no way to start importing portions of the WebCrypto conformance tests to guide development?

I did not spend a lot of time looking at the WPTs yet or how to include them. I never worked with WPTs before and I am not sure how to run these tests outside of browsers.

Is Babel really necessary? [...] can you justify its inclusion?

Probably not, I just prefer the ESM syntax over CJS. I assume you are suggesting to convert the package to CJS instead of "native" ESM?

Or, pull the nodejs/node rules if you'd prefer them.

Good idea, I'll do that.

[gengjiawen]

Probably not, I just prefer the ESM syntax over CJS.

+1 on ESM. But we can use esm with --experimental-modules or try Typescript use "module": "commonjs" as an option.

[rvagg]

Probably not, I just prefer the ESM syntax over CJS. I assume you are suggesting to convert the package to CJS instead of "native" ESM?

My suggestion would be CJS, just plain, no flags, no compiling, .js files that can be run straight. We're just talking about swapping import for require after all and we get to save huge complexity and dependencies.

[ljharb]

Definitely please do not rely on anything that requires a node flag :-)

[tniessen]

Okay, I removed babel, rewrote the code in CJS, and added Node.js linting rules. Hope I didn't miss anything.

[mcollina]

Maybe we should integrate Travis and get a CI run?

[tniessen]

There is a .travis.yml in the first commit, maybe someone with admin access can enable Travis? Will Travis use the config file from the PR or do we need to merge first?

[targos]

Travis is enabled

[targos]

I think it will use the config file from the PR but you need to push something to trigger it

[joyeecheung]

I did a POC of porting the WPT runner in Node.js core for the WHATWG stream repo before, and using node-core-utils to pull down the resources from the WPT repo, it might be useful: instructions are in the OP of https://github.com/nodejs/whatwg-stream/pull/2

The WPT runner was updated a bit since that but it should be only more independent of Node.js internals now.

[targos]

@panva I don't know how to do it from a fork

[panva]

@panva I don't know how to do it from a fork

Yeah I realized its a fork so I removed my message :) you were too quick to see it :)

[tniessen]

I force-pushed the same commit with a different timestamp and it triggered Travis, thanks for setting it up @targos! :-)

[joyeecheung]

On the ESM v.s. CJS issue: although this is initially just exploring implementing WebCrypto outside of core, note that we do not support ESM internally in Node.js core. The internal modules are not compiled in the same way as normal user land CJS modules either. To include a ESM in core we need to either transpile it to CJS (and place the transpiled files under either deps or lib depending on the maintenance strategy) or write another special loader for internal ESM.

[tniessen]

Good point, if we intend to add this implementation to core at some point, that would be a problem as well. I think we agree that CJS is the better choice for now, even though it has some downsides.

[tniessen]

Thanks for the thorough review, Sam!