beyerleinf / esbuild-azure-functions

A ✨blazingly fast✨ builder for Azure Functions powered by esbuild.
MIT License
12 stars 2 forks source link

Build not including global import of 'reflect-metadata' in nestjs project #17

Open tyler-suderman-kr opened 2 years ago

tyler-suderman-kr commented 2 years ago

I've been working with this library and have had good luck getting everything to build properly with the help of your documentation, but we're running into an issue with a dependency we import as a peer dependency of nest, import 'reflect-metadata'. We would like this to be included in our outDir but we're unable to pull it in. The function app bails on start with error, 'Reflect.getMetadata is not a function'.

One way to get around the issue is to leverage the build plugin, EsmExternalsPlugin. Locally this works when the node_modules folder is in scope because the dependency is being sourced externally. But this solution won't work because we need to be able to reference the dependency from within out outDir when zipping and sending to Azure for deployment.

Do you have any information regarding if either this library or the global import itself is the issue here? Any suggestions on how to proceed?

beyerleinf commented 2 years ago

Hi @tyler-suderman-kr, thanks for reporting this! Could you maybe link to your project or create a small reproduction so I can verify this?

I can at least tell you that it's possible as I have a private project where I implemented decorators for my Azure Functions and reflect-metadata get included without any issues

tyler-suderman-kr commented 2 years ago

Happy to work on sharing an example... are you saying your project also includes the import 'reflect-metadata'?

Either way, do you mind pointing me to your configuration for the project you have implementing decorators?

beyerleinf commented 2 years ago

Jup, I have some decorators that look like this, so should be a similar use case to your project:

// decorator.ts

import 'reflect-metadata';
import { HTTP_CODE_METADATA } from '../../constants';

export function HttpCode(code: number) {
  return (target: any, key: string, descriptor: PropertyDescriptor) => {
    Reflect.defineMetadata(HTTP_CODE_METADATA, code, descriptor.value);
  };
}

For my build config, I have nothing special:

// build.mjs

await build({
  project: '.',
  advancedOptions: {
    enableDirnameShim: true,
    enableRequireShim: true,
  },
  clean: true,
  logLevel: 'verbose'
});
tyler-suderman-kr commented 2 years ago

We've discovered the issue and it may be worth adding to the documentation. We are importing an internal dependency that also relies on 'reflect-metadata'. Usually global imports provide the necessary access by including the dependency at the top in the build. But there is a known issue with esbuild where code splitting produces incorrect ordering of imports for projects with multiple entry points.

tyler-suderman-kr commented 2 years ago

We'll be following this issue and likely implement your library dependent on its being addressed. Thank you for the context and helping us narrow down our search!

beyerleinf commented 2 years ago

Very interesting and thanks for investigating further! If you could give me a minimal structure that can reproduce this, I'll add it to the docs until the issue is fixed (I'm actually transferring the docs to GitBook right now, so let me know if you think anything else is missing).

I'm curious how your setup looks, if your interested, this is what my project looks like (it's a Nx monorepo I'm also using to test the Nx integration I'm currently building)

image

For now, you could turn off code splitting to benefit from the faster builds and probably still keep slightly smaller bundle size until the issue is fixed.

tyler-suderman-kr commented 2 years ago

We can also patch the issue by importing 'reflect-metadata' into the context of the code which lands out-of-scope on build as a workaround. I'll work on more context

tyler-suderman-kr commented 2 years ago

better reference to ongoing issue: https://github.com/evanw/esbuild/issues/16

tyler-suderman-kr commented 2 years ago

deps

This is a not super pretty but hopefully sufficient breakdown the the dependency structure and issue we're encountering. The issue is documented on the code splitting section of the esbuild docs also.

tyler-suderman-kr commented 2 years ago

In regards to other missing pieces of the documentation, this is the only major snag we hit that wasn't addressed in your triaging section

beyerleinf commented 2 years ago

After some time I managed to get a test repro outputting what I hope is similar to what you get. However, for me the code still works. Can you verify that I did it right? I want to create a small example for the docs...

https://github.com/beyerleinf/esbuild-azure-functions/tree/12-update-docs/test

tyler-suderman-kr commented 2 years ago

The setup is more complicated that this. We manage two .git projects in the visual above. One of the repos is the 'Workspace' with each of the nested function apps being inside the repo, but all having their own package.json files to manage the deps for each separately. The other repo is the common-utilities, which has both .git and package.json on the same, top level in the project.

In order to avoid having to manage multiple instances of nestJS and reflect-metadata, we bring in common-utilities as a dependency, and then tell common-utilities to use whatever version of Nest and Reflect that comes with the parent.

beyerleinf commented 2 years ago

I'm leaving out the example for now 😄 Let's hope it get's fixed anytime soon.