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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
tyler-suderman-kr opened this issue Oct 12, 2022 · 13 comments
Assignees
Labels
3rd party An issue that depends on a 3rd party library to fix something needs triage An issue that needs verification

Comments

@tyler-suderman-kr
Copy link

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 beyerleinf self-assigned this Oct 12, 2022
@beyerleinf beyerleinf added the needs triage An issue that needs verification label Oct 12, 2022
@beyerleinf
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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

@beyerleinf beyerleinf added the 3rd party An issue that depends on a 3rd party library to fix something label Oct 13, 2022
@tyler-suderman-kr
Copy link
Author

better reference to ongoing issue: evanw/esbuild#16

@tyler-suderman-kr
Copy link
Author

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rd party An issue that depends on a 3rd party library to fix something needs triage An issue that needs verification
Projects
None yet
Development

No branches or pull requests

2 participants