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

Error with custom entryPoint in adapter-node #2481

Closed
oskarhane opened this issue Sep 23, 2021 · 6 comments · Fixed by #2482
Closed

Error with custom entryPoint in adapter-node #2481

oskarhane opened this issue Sep 23, 2021 · 6 comments · Fixed by #2482
Labels
bug Something isn't working pkg:adapter-node
Milestone

Comments

@oskarhane
Copy link

oskarhane commented Sep 23, 2021

Describe the bug

When setting up adapter-node I'm configuring a custom entry point copied from the example here: https://github.com/sveltejs/kit/tree/master/packages/adapter-node#middleware but I run into this error when I try to start the server with node build: Dynamic require of "buffer" is not supported.

The error message points to the jsonwebtoken package which I use on some node only endpoints.
But, this is not specific to this package, it's the same with dotenv and other node packages.

If I skip the entryPoint config in the adapter setup, it works.

Reproduction

Minimal repro repo: https://github.com/oskarhane/sveltekit-adapter-node-bug-repro

npm ci

All I've done there is to npm init svelte@next, installing @sveltejs/adapter-node, jsonwebtoken and polka and configured the adapter after creating a single request handler that uses the jwt package.

npm run build
node build

Edit (toggle the comments) the adapter config in svelte.config.js and try again to have it work / not work.

Logs

▶ node build   
file:///Users/oskarhane/Development/tmp/sk-bug/build/index.js:13
  throw new Error('Dynamic require of "' + x + '" is not supported');
        ^

Error: Dynamic require of "buffer" is not supported
    at file:///Users/oskarhane/Development/tmp/sk-bug/build/index.js:13:9
    at node_modules/safe-buffer/index.js (file:///Users/oskarhane/Development/tmp/sk-bug/build/index.js:33:18)
    at __require2 (file:///Users/oskarhane/Development/tmp/sk-bug/build/index.js:16:44)
    at node_modules/jws/lib/sign-stream.js (file:///Users/oskarhane/Development/tmp/sk-bug/build/index.js:564:19)
    at __require2 (file:///Users/oskarhane/Development/tmp/sk-bug/build/index.js:16:44)
    at node_modules/jws/index.js (file:///Users/oskarhane/Development/tmp/sk-bug/build/index.js:744:22)
    at __require2 (file:///Users/oskarhane/Development/tmp/sk-bug/build/index.js:16:44)
    at node_modules/jsonwebtoken/decode.js (file:///Users/oskarhane/Development/tmp/sk-bug/build/index.js:777:15)
    at __require2 (file:///Users/oskarhane/Development/tmp/sk-bug/build/index.js:16:44)
    at node_modules/jsonwebtoken/index.js (file:///Users/oskarhane/Development/tmp/sk-bug/build/index.js:2835:15)

System Info

System:
    OS: macOS 11.5.2
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 3.38 GB / 32.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 14.17.4 - ~/.nvm/versions/node/v14.17.4/bin/node
    Yarn: 1.22.11 - ~/.nvm/versions/node/v14.17.4/bin/yarn
    npm: 6.14.14 - ~/.nvm/versions/node/v14.17.4/bin/npm
  npmPackages:
    @sveltejs/adapter-node: ^1.0.0-next.49 => 1.0.0-next.49 
    @sveltejs/kit: next => 1.0.0-next.171 
    svelte: ^3.34.0 => 3.38.3 

Severity

blocking all usage of SvelteKit

Additional Information

No response

@Conduitry Conduitry added bug Something isn't working pkg:adapter-node and removed bug Something isn't working labels Sep 23, 2021
@Conduitry
Copy link
Member

The generated build/index.js file doesn't appear to properly have ./middlewares.js as an eternal dependency - it's getting bundled. This is a problem because only build/middlewares.js gets the needed require shim.

@benmccann As the author of #2414, do you have any ideas?

@benmccann benmccann added this to the 1.0 milestone Sep 23, 2021
@Conduitry
Copy link
Member

If I update the regex at

build.onResolve({ filter: /^\.\/middlewares\.js$/ }, () => ({ external: true }));
to match any file ending in /middlewares.js as external, this appears to work, but that's too heavy-handed of a solution. I'm not sure exactly what this is trying to work around, but apparently something is passing a different path to this onResolve hook.

@benmccann
Copy link
Member

The only thing I did in #2414 was let you modify the entry point. I'm not familiar with esbuild at all, so don't know what all this other stuff is about

@Conduitry
Copy link
Member

Proof-of-concept solution (I think):

diff --git a/packages/adapter-node/index.js b/packages/adapter-node/index.js
index cc2dd47d..4bc9e805 100644
--- a/packages/adapter-node/index.js
+++ b/packages/adapter-node/index.js
@@ -7,7 +7,7 @@ import {
 	statSync,
 	writeFileSync
 } from 'fs';
-import { join } from 'path';
+import { join, resolve } from 'path';
 import { pipeline } from 'stream';
 import glob from 'tiny-glob';
 import { fileURLToPath } from 'url';
@@ -81,7 +81,6 @@ export default function ({
 				entryPoints: [entryPoint],
 				outfile: join(out, 'index.js'),
 				bundle: true,
-				external: ['./middlewares.js'], // does not work, eslint does not exclude middlewares from target
 				format: 'esm',
 				platform: 'node',
 				target: 'node12',
@@ -90,8 +89,12 @@ export default function ({
 					{
 						name: 'fix-middlewares-exclude',
 						setup(build) {
-							// Match an import called "./middlewares.js" and mark it as external
-							build.onResolve({ filter: /^\.\/middlewares\.js$/ }, () => ({ external: true }));
+							// Match an import of "middlewares.js" and mark it as external
+							build.onResolve({ filter: /\/middlewares\.js$/ }, ({ path, resolveDir }) => {
+								if (resolve(resolveDir, path) === resolve(out, 'middlewares.js')) {
+									return { external: true };
+								}
+							});
 						}
 					}
 				]

This uses a regex to filter out just import paths ending in /middlewares.js and then, to avoid false positives, it actually checks what the file would resolve to before indicating that this is to be marked external.

If I'm understanding esbuild's architecture correctly, the regex gets turned into a Go regex, and we only have to hop over into JS when it's matched. So, it's worthwhile to try to filter out as much as possible in the regex, rather than in the JS callback.

@Conduitry
Copy link
Member

I'm not wild about the resolve(resolveDir, path) === resolve(out, 'middlewares.js') check, but I'm not sure what would be a better way to handle this. When this hook is called for the middlewares file, this is what it's passed and what we have to work with:

{
  path: '../build/middlewares.js',
  importer: '/some/path/sveltekit-adapter-node-bug-repro/src/server.js',
  namespace: 'file',
  resolveDir: '/some/path/sveltekit-adapter-node-bug-repro/src',
  kind: 'import-statement',
  pluginData: undefined
}

@binajmen
Copy link

binajmen commented Sep 15, 2022

@oskarhane Did you solve your issue? I'm having the exact same issue while deploying on Fly.io, that is I'm using jsonwebtoken in two places: hooks.server.ts and /some/routes/+page.server.ts.

@Conduitry Could it be your fix is no more working since the latest changes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pkg:adapter-node
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants