Split programming model out of worker

[ejizba]

The goal is essentially to split our code into two repos, while maintaining existing behavior:

• azure-functions-nodejs-library: This is a new repo I created based on offline discussion about the name. It will have the same git history as the worker repo up until this point, but will diverge moving forward. It will ship the @azure/functions npm package, which will include the entire programming model moving forward (not just the types, although it'll still have that). See PR here: Split programming model out of worker (library side) azure-functions-nodejs-library#1
• azure-functions-nodejs-worker: Our existing repo. NOTE: I won't be able to merge this PR until the new npm package exists.

Most important files to review:

• types-core/index.d.ts: This defines the API between the two pieces
• src/eventHandlers/InvocationHandler.ts: This is the main file that uses the new api

Fixes #568

[hossam-nasr]

Overall, I think I follow what the code is doing, and no major issues there, but I'm not sure what goal this achieves?

As far as I understand, this so far only includes modifying function invocation, not function loading and indexing, which was the major difference and advantage of having new programming models. Also, I'm not sure what benefit allowing programming models to define invokeFunction() and getResponse() achieves? For example, what's an example of a programming model that's meaningfully different from the default one that would leverage those methods?

[hossam-nasr]

Shouldn't we import this as the npm package?

[hossam-nasr]

Not also overriding the context here?

[ejizba]

That is correct. Similar to the debate we had for hooksData (#601), I don't want people overriding context, just modifying it. It is a class we as the worker created, and it has too much important information shared across our worker, the library package, and the user's app to be overwritten.

By contrast, the inputs and callback were created directly by the user and the programming model so I think it makes sense to let them overwrite it. Also they are just much simpler than context

[hossam-nasr]

What do you mean? I'm talking about the plain context, which is created by the invocation model, not the coreCtx. This is one is created and managed by the programming model, so it makes sense if it wants to override it? How is it used by the worker?

[ejizba]

Yes you are correct it is the plain context not coreCtx so the worker does not create or use it directly. However, my larger point is still accurate in the sense that it has too much important information and should not be overwritten. The hooks are not a part of the programming model, so it would not be a case of the programming model overriding its own context.

[ejizba]

This work is an important stepping stone on the way to the new indexing stuff. At its core, it is purely engineering work with no functional difference to the user. Perhaps that's why the "goal" isn't clear, but I did it that way because I think that makes it a lot easier to review. I try to separate "engineering PRs that affect a lot of files" from "feature PRs". The indexing stuff is tracked by #569 and it will be a separate PR - that PR will be a lot smaller (comparatively 😅), but it will also be a lot more complex because it's introducing new features.

Please read through #568 if you haven't already. I think it answers a lot of your questions. I think the biggest piece you're missing is the type of code we want in each place. We want simple code with no chance of bugs in the worker because it takes so long to ship. We want more complex code (meaning more helpful code, but also code with a higher chance of bugs) in the npm package. Also keep in mind that people can build off of our programming model npm package. So even though I'm moving FunctionInfo (and other classes) into the npm package, people can still share that code and build off of it. In other words, a "new" programming model doesn't have to start from scratch - it can just wrap the one we provide in the npm package.

[ejizba]

Sent a meeting invite to discuss offline - I think that'll be easiest