Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Node.js Functions programming model 馃殌 #480

Closed
ejizba opened this issue Nov 29, 2021 · 17 comments
Closed

Improve Node.js Functions programming model 馃殌 #480

ejizba opened this issue Nov 29, 2021 · 17 comments
Assignees
Labels
enhancement P1 v4 model 馃殌 Related to the new V4 programming model
Milestone

Comments

@ejizba
Copy link
Contributor

ejizba commented Nov 29, 2021

Goals

  1. Be idiomatic for Node.js developers. Users should be able to look at the code and immediately know what's going on
  2. Let users define their own file structure (related to Allow discovery of functions in nested directories聽azure-functions-host#5373)
  3. Let users define function metadata in js/ts files instead of json
  4. Take advantage of types as much as possible, including for JavaScript users (i.e. VS Code can provide Intellisense for JavaScript based on the TypeScript types as long as it can find them)
  5. Model after an existing popular/modern framework (Fastify, Express, Firebase, Remix, etc.), but ideally be flexible enough to allow for other frameworks to be used

Plan of action

Moved to here: https://github.com/Azure/azure-functions-nodejs-worker/wiki/Roadmap#new-programming-model

Prototypes

See this repo for the example prototype: https://github.com/ejizba/func-nodejs-prototype

@aaronpowell
Copy link

aaronpowell commented Feb 11, 2022

One of the hardest things to handle is bindings - there's nothing like .NET attributes that you can leverage, so there's really two options IMO - object literal or a fluent API.

Here's some rough prototype ideas:

import app from "@azure/functions";

// object literal
app.register(async (context: FunctionContext) => {
  context.res = {
    body: { hello: "world" }
  };
}, {(
  bindings: [{
    type: "http",
    direction: "in",
    name: "req",
    methods: ["get", "post"]
  }, {
    type: "http",
    direction: "out",
    name: "res"
  }],
  route: "/echo"
});

// fluent API
app.register(async (context: FunctionContext) => {
  context.res = {
    body: { hello: "world" }
  }
}).binding({
    type: "http",
    direction: "in",
    name: "req",
    methods: ["get", "post"]
}).binding({
    type: "http",
    direction: "out",
    name: "res"
}).route("echo");

Both of those are a "low level API" and I don't see why you couldn't have something more like app.registerHttp(handler: AzureFunction, route: string, methods?: ("get" | "post" | "patch" | ...)[], additionalBindings?: FunctionBinding[]) as a method which does the sensible defaults but allows more stuff to be added.

I've also only done HTTP here, but there's nothing HTTP-centric around this model.

@aaronpowell
Copy link

I'd be looking at libraries like Fastify/Express/etc. as inspirations for how to create the API surface area, but not try and mimic them completely as they are centred so much around HTTP only, when you want to make a programming model that fits nicely for other bindings too.

@ryancole
Copy link

ryancole commented Mar 9, 2022

I feel honored to be in the bullet list of items that triggers this issue. JK. JK. :P

@sinedied
Copy link
Member

sinedied commented Mar 9, 2022

With the prominence of TypeScript, I would suggest thinking of how the new API model could leverage TS to make it a first class citizen. For example, I would push forward TS decorators as a way to define bindings and other functions settings. I know decorators' TC39 proposal is currently only stage 2, and TS decorators are already diverging a bit from that proposal. But as syntactic sugar, they're unmatched and AFAIK all most popular TS-first frameworks tools use decorators: Angular, NestJS, TypeORM, TypeGraphQL, Loopback, Ts.ED...

This is not necessarily incompatible with @ejizba's prototype approach, but could come in addition to the regular syntax:

import { Http, Get, AuthLevel, FunctionContext } from '@azure/functions';

@Http('/hello')
@Get()
@AuthLevel('anonymous')
export async function ({ req, res }: FunctionContext) {
  res.body = 'Hello World!';
}

One key point is see would be to provide sensible defaults to reduce boilerplate (as @aaronpowell said), like in my example above the @Http() decorator would set in/out bindings associated to req and res, which is probably the most common use case.

Such an approach regarding default would also work with plain JS using functions:

import { app, FunctionContext } from '@azure/functions';

app.http('/hello', { methods: ['get'], authLevel: 'anonymous' },
  async ({ req, res }: FunctionContext) => {
    res.body = 'Hello World!';
  });

Or with a fluent api:

import { app, FunctionContext } from '@azure/functions';

app.http('/hello')
  .methods(['get'])
  .authLevel('anonymous')
  .register({ req, res }: FunctionContext) => {
    res.body = 'Hello World!';
  });

Of course, the full verbose would still be accessible, but reducing the amount of boilerplate for common use cases is something to not be overlooked.

@aaronpowell
Copy link

I would push forward TS decorators as a way to define bindings and other functions settings

I'm going to disagree with this. From the conversations that I've had with the TypeScript team, I felt that they were not really encouraging new projects to use the decorators that are in TypeScript because, as you pointed out, they aren't in line with the spec and if/when decorators do make it into the spec, they won't be able to directly port over the current implementation, so they'll have to have two different implementations within the language itself.

@sinedied
Copy link
Member

sinedied commented Mar 9, 2022

The first goal is to be idiomatic for Node.js dev, and the fact is that decorators are hugely popular in all TS-first Node.js libs. This is what I think is the most important point here.

Now, I don't say that the API should be built exclusively around decorators, but we can use them as syntactic sugar around the base API. When/if the new decorators spec will be defined, the whole TS ecosystem would have to think about a migration path, and the TS compiler won't drop support for the old spec the next day.

@ejizba
Copy link
Contributor Author

ejizba commented Mar 9, 2022

I understand there may be technical reasons decorators would be difficult for us (as @aaronpowell said), but I do plan to explore that option and have at least one prototype with decorators. I make no promises, but I think it's worth considering

@anfibiacreativa
Copy link
Member

The first goal is to be idiomatic for Node.js dev, and the fact is that decorators are hugely popular in all TS-first Node.js libs. This is what I think is the most important point here.

Now, I don't say that the API should be built exclusively around decorators, but we can use them as syntactic sugar around the base API. When/if the new decorators spec will be defined, the whole TS ecosystem would have to think about a migration path, and the TS compiler won't drop support for the old spec the next day.

I support exploring the decorators option. Makes a lot of sense to me.

@aaronpowell
Copy link

With decorators progressing to Stage 3 last week, it's probably more viable to consider, but we'd want to leverage the design of decorators that's spec'ed out rather than the one in TypeScript today (and that might make things harder).

@sedot42
Copy link

sedot42 commented May 24, 2022

Wouldn't TS decorators be just a nice potential feature of the new programming model? Imo it sounds like a good compromise to keep this feature in mind and focus on getting the basics of a good programming model right.

I like the brevity of the high-level API form suggested here by @sinedied, leveraging the low-level API proposal of @aaronpowell.

import { app, FunctionContext } from '@azure/functions';

app.http('/hello', { methods: ['get'], authLevel: 'anonymous' },
  async ({ req, res }: FunctionContext) => {
    res.body = 'Hello World!';
  });

@aaronpowell
Copy link

I like the idea of decorators in principle but until there's support for the proposal that's going forward not the legacy one in TypeScript and Babel/swc/etc. I see it as being a really hard thing to support as a first-class citizen.

ejizba added a commit to Azure/azure-functions-nodejs-library that referenced this issue Aug 20, 2022
A first pass that includes support for http, timer, and storage, plus a 'generic' option that could be used for anything.

One of the biggest pieces missing is that this does not touch the http request or response types yet.

Related to Azure/azure-functions-nodejs-worker#480
@ChuckJonas
Copy link

@aaronpowell IMO the object literal approach is where you should start.

Once you have that, community members interested in decorators could roll their own package. Allowing functions to be registered via code really opens the door to a lot of potential solutions.

IMO, there was only support for fluent API, it makes it a bit harder to extend.

@ejizba
Copy link
Contributor Author

ejizba commented Sep 2, 2022

Hi folks! We've actually narrowed it down to one option, and it's a working prototype and everything. More details here if you're interested in giving some feedback: https://aka.ms/AzFuncNodeV4

@ChuckJonas in terms of decorators, it's definitely been on the back burner for now. We want to prototype it still, but we've been focused on a non-decorators approach as described in the link above.

@ChuckJonas
Copy link

ChuckJonas commented Sep 4, 2022

@ejizba just gave it a review. I like where this is going!

Honestly the thing I'm most excited about is just being able to specify whatever folder structure fits the project best and not have every function as a folder in the root.

Our project is using OpenAPI specs to define our API contracts along side each function. We use openapi-typescript to get accurate request typings and validate all request using against the spec using ajv.

Overall, I think it's a pretty nice approach, but the limitations around function registration really make it harder to maintain than it would otherwise be.

Some feedback below:

Define function in code

  1. Is the first parameter the route? Or just the function name? I'm hoping it's the former as this would make it similar to other popular JS frameworks.

  2. Do you have an example Service bus triggered function?

Simplified context, inputs, and outputs

The context object has been simplified to reduce duplication and make it easier to write unit tests

That will be very welcome. Been burned by missing the context duplication in our test setup a couple times now.

Are there any plans to improve Service Bus IN/OUT formats? Guessing that might be outside the scope of this project, but I have two major problems with how service bus bindings work in AZ Function:

  1. in: input provides only the message.body. You have to go to the binding read the message properties / applicationProperties.

  2. out As far as I can tell, there are no ways to set anything other than the message body. This makes the out bindings a no-go for all but the simplest use cases (no sessionId, applicationProperties, etc)

Create a test context

Will be nice to have this out of the box... We basically rolled out our version of something very similar in our current project.

Question: Rules around function registration

Just curious in general how the function registration works...

  • Can functions be conditional registered?
  • Is there anything that stops a function from registering another function?

PS:
IMO, decorators don't bring much other than just making the code read closer what whats done in C#. It feels like the ts community has shifted away from OOO in favor of more functional code (decorators are only supported on classes). Current approach seems more flexible and composable.

@ejizba
Copy link
Contributor Author

ejizba commented Sep 6, 2022

@ChuckJonas thanks for the feedback! I think I've covered everything:

Is the first parameter the route? Or just the function name? I'm hoping it's the former as this would make it similar to other popular JS frameworks.

It's the function name. Created this issue to continue the discussion on this: Azure/azure-functions-nodejs-library#17

Do you have an example Service bus triggered function?

Yeah here are my example functions with a few service bus ones: https://github.com/ejizba/func-nodejs-prototype/tree/main/src/functions. Make sure to read the readme of that repo if you want to know how to try it out

Are there any plans to improve Service Bus IN/OUT formats?

No there are not. Agreed it's probably out of scope, as I think your problems are common across all languages, right? Regardless, feel free to file a separate issue if you want us to follow up

Can functions be conditional registered?

Yes, for example I added some conditions to only register a function if the respective environment variable is set in my example prototype. (see here)

Is there anything that stops a function from registering another function?

That is not possible. If you try it, you will get an error like "A function can only be registered during app startup."

IMO, decorators...

I've created a separate issue to start tracking any decorators work/discussion: Azure/azure-functions-nodejs-library#18

@ChuckJonas
Copy link

ChuckJonas commented Sep 28, 2022

@ejizba Ran into an issue today with distributed tracing that I was wondering if you guys would be solving via this initiative.

The problem is outlined here.

IDK if maybe this could be solved with the new hooks. I am using appinsights wrapped context, but you end up needing to rewrite all the context.log functionality because its operation id doesn't get updated.

export default async function contextPropagatingHttpTrigger(context, req) {
  // overwrite with the proper traceparent from the incoming SB message. 
  const sbTraceParent = context.bindingData.applicationProperties['diagnostic-Id'];
  if (sbTraceParent) {
    context.traceContext.traceparent = sbTraceParent;
  }

  const correlationContext = appInsights.startOperation(context, req) as CorrelationContext;

  // Wrap the Function runtime with correlationContext
  return appInsights.wrapWithCorrelationContext(async () => {
    const startTime = Date.now(); // Start trackRequest timer

    try {
      // operation_Id matches SB 'diagnostic-Id'
      appInsights.defaultClient.trackTrace({
        message: 'Correct Trace Context',
      });

      // operation_Id is the original function traceparent
      context.log('Incorrect Trace Context');

      return await trigger(context, req);

    } catch (e) {
      context.log.error(e);
      throw e;
    } finally {
      // Track Request on completion
      appInsights.defaultClient.flush();
    }
  }, correlationContext)();
}

@ejizba
Copy link
Contributor Author

ejizba commented Mar 27, 2023

We're in public preview! 馃帀 Try it out now and let us know what you think - read the blog post here: https://techcommunity.microsoft.com/t5/apps-on-azure-blog/azure-functions-version-4-of-the-node-js-programming-model-is-in/ba-p/3773541

I'm going to close this issue and we will track any GA work with its own individual issue

@ejizba ejizba closed this as completed Mar 27, 2023
@ejizba ejizba unpinned this issue Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement P1 v4 model 馃殌 Related to the new V4 programming model
Projects
None yet
Development

No branches or pull requests

7 participants