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

add support to vercel edge runtime #141

Conversation

bernatvadell
Copy link

When compiling new nextjs projects it gives the following error:

../../node_modules/.pnpm/reflect-metadata@0.1.13/node_modules/reflect-metadata/Reflect.js
Dynamic Code Evaluation (e. g. 'eval', 'new Function', 'WebAssembly.compile') not allowed in Edge Runtime 
Learn More: https://nextjs.org/docs/messages/edge-dynamic-code-evaluation

Import trace for requested module:
../../node_modules/.pnpm/reflect-metadata@0.1.13/node_modules/reflect-metadata/Reflect.js
../../packages/sdk/dist/esm/index.js

This is due to the dynamic evaluation of this piece of code in Reflect.ts:

Function("return this;")();

To make it compatible, I've changed it to:

(() => { return this; })();

With this simple change, the project already compiles correctly.

@ljharb
Copy link

ljharb commented Aug 1, 2023

Those aren’t the same; using Function is the only reliable way to get to the global, since it’s in sloppy mode.

@rezonant
Copy link

rezonant commented Aug 1, 2023

Fascinating. Here's a write-up discussing the difficulties of accessing globalThis portably (even on runtimes that do not have globalThis).

https://mathiasbynens.be/notes/globalthis

Could we use the same accessor trick to accomplish this while avoiding eval/Function constructor?

@ljharb
Copy link

ljharb commented Aug 1, 2023

Realistically this package should probably use https://npmjs.com/globalthis instead, and then your own bundler could alias it out with whatever env-specific mechanism you need.

@rezonant
Copy link

rezonant commented Aug 1, 2023

I agree it would be better to polyfill outside this module, but we can't just put a require() in there (or include for that matter) without limiting the scope in which the package can be loaded.

I personally don't have any qualms about simply requiring users to polyfill globalThis themselves if they need to, as long as it would be a major version bump.

@rbuckton
Copy link
Owner

Is this a build failure or a runtime failure? If it's a runtime failure, providing a globalThis polyfill would avoid any dynamic evaluation. If it's a build failure, is there any way to exclude reflect-metadata from this restriction?

I don't believe I can drop the dynamic evaluation mechanism to acquire a reference to the global object without potentially breaking consumers. However, in reflect-metadata@0.2.0-pre.0 I recently introduced a "lite" mode that drops the internal Map/Set polyfill, which is accessed via an alternative import of import "reflect-metadata/lite";. I could see modifying "lite" mode to elide dynamic evaluation with the expectation that any necessary polyfills are already installed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants