payload icon indicating copy to clipboard operation
payload copied to clipboard

node.fetch ESM import issue in getFileByURL.js

Open cbratschi opened this issue 1 year ago • 20 comments

Link to reproduction

private repo

Describe the Bug

Getting the following error in Payload 2.4.0 while trying to build the types:

/.../node_modules/payload/dist/uploads/getFileByURL.js:121
undefined
      ^

Error [ERR_REQUIRE_ESM]: require() of ES Module /.../node_modules/node-fetch/src/index.js from /.../node_modules/payload/dist/uploads/getFileByURL.js not supported.
Instead change the require of index.js in /.../node_modules/payload/dist/uploads/getFileByURL.js to a dynamic import() which is available in all CommonJS modules.
    at Object.newLoader [as .js] (/.../node_modules/pirates/lib/index.js:121:7)
    at Object.<anonymous> (/.../node_modules/payload/dist/uploads/getFileByURL.js:11:59)
    at Object.newLoader [as .js] (/.../node_modules/pirates/lib/index.js:121:7) {
  code: 'ERR_REQUIRE_ESM'
}

Node.js v20.10.0

npm ERR! Lifecycle script `payload:types` failed with error: 

To Reproduce

npm run payload:types is failing after upgrading to 2.4.0. All packages are the latest versions.

Payload Version

2.4.0

Adapters and Plugins

db-mongodb, bundler-webpack, live-preview

cbratschi avatar Dec 07 '23 16:12 cbratschi

According to node-fetch the following should be used on CJS:

// mod.cjs
const fetch = (...args) => import('node-fetch').then(({default: fetch}) => fetch(...args));

https://www.npmjs.com/package/node-fetch

The CJS and ESM syntax is different and TypeScript code gets here transpiled to CJS syntax which is wrong.

General question: why not using Node.js' fetch on 20.x?

cbratschi avatar Dec 07 '23 16:12 cbratschi

Fails here: https://github.com/payloadcms/payload/blob/48f1299fcba3e3811c6a7f31499f238537f9a5e3/packages/payload/src/uploads/getFileByURL.ts#L1

cbratschi avatar Dec 07 '23 16:12 cbratschi

Patched the transpiled getFileByURL.js file:

"use strict";
Object.defineProperty(exports, "__esModule", {
    value: true
});
Object.defineProperty(exports, "default", {
    enumerable: true,
    get: function() {
        return _default;
    }
});
//const _nodefetch = /*#__PURE__*/ _interop_require_default(require("node-fetch"));
const _path = /*#__PURE__*/ _interop_require_default(require("path"));
function _interop_require_default(obj) {
    return obj && obj.__esModule ? obj : {
        default: obj
    };
}
const getFileByURL = async (url)=>{
    if (typeof url === 'string') {
        const res = await fetch(url, {
            headers: {
                'Content-Type': 'application/json'
            },
            method: 'GET'
        });
        const data = await res.buffer();
        const name = _path.default.basename(url);
        return {
            name,
            data,
            mimetype: res.headers.get('content-type') || undefined,
            size: Number(res.headers.get('content-length')) || 0
        };
    }
};
const _default = getFileByURL;

cbratschi avatar Dec 07 '23 16:12 cbratschi

See discussion here: https://github.com/node-fetch/node-fetch/issues/1279#issuecomment-1275933370

cbratschi avatar Dec 07 '23 16:12 cbratschi

That file was recently introduced by https://github.com/payloadcms/payload/pull/4170

denolfe avatar Dec 08 '23 12:12 denolfe

Having updated to Payload 2.4.0, I'm currently unable to npm run dev.

I'm currently on Node 20.8.0.

[nodemon] 2.0.22
[nodemon] to restart at any time, enter `rs`
[nodemon] watching path(s): *.*
[nodemon] watching extensions: ts
[nodemon] starting `ts-node src/server.ts -- -I`

/data/projects/ac/payload/node_modules/ts-node/dist/index.js:701
            return old(m, filename);
                   ^
Error [ERR_REQUIRE_ESM]: require() of ES Module /data/projects/ac/payload/node_modules/node-fetch/src/index.js from /data/projects/ac/payload/node_modules/payload/dist/uploads/getFileByURL.js not supported.
Instead change the require of index.js in /data/projects/ac/payload/node_modules/payload/dist/uploads/getFileByURL.js to a dynamic import() which is available in all CommonJS modules.
    at require.extensions.<computed> [as .js] (/data/projects/ac/payload/node_modules/ts-node/dist/index.js:701:20)
    at Object.<anonymous> (/data/projects/ac/payload/node_modules/payload/dist/uploads/getFileByURL.js:11:59)
    at require.extensions.<computed> [as .js] (/data/projects/ac/payload/node_modules/ts-node/dist/index.js:701:20)
    at Object.<anonymous> (/data/projects/ac/payload/node_modules/payload/dist/uploads/generateFileData.js:21:62)
    at require.extensions.<computed> [as .js] (/data/projects/ac/payload/node_modules/ts-node/dist/index.js:701:20)
    at Object.<anonymous> (/data/projects/ac/payload/node_modules/payload/dist/collections/operations/create.js:22:27)
    at require.extensions.<computed> [as .js] (/data/projects/ac/payload/node_modules/ts-node/dist/index.js:701:20)
    at Object.<anonymous> (/data/projects/ac/payload/node_modules/payload/dist/collections/requestHandlers/create.js:14:56)
    at require.extensions.<computed> [as .js] (/data/projects/ac/payload/node_modules/ts-node/dist/index.js:701:20)
    at Object.<anonymous> (/data/projects/ac/payload/node_modules/payload/dist/collections/buildEndpoints.js:21:56)
    at require.extensions.<computed> [as .js] (/data/projects/ac/payload/node_modules/ts-node/dist/index.js:701:20)
    at Object.<anonymous> (/data/projects/ac/payload/node_modules/payload/dist/collections/initHTTP.js:16:64)
    at require.extensions.<computed> [as .js] (/data/projects/ac/payload/node_modules/ts-node/dist/index.js:701:20)
    at Object.<anonymous> (/data/projects/ac/payload/node_modules/payload/dist/initHTTP.js:15:58)
    at require.extensions.<computed> [as .js] (/data/projects/ac/payload/node_modules/ts-node/dist/index.js:701:20)
    at Object.<anonymous> (/data/projects/ac/payload/node_modules/payload/dist/index.js:22:19)
    at require.extensions.<computed> [as .js] (/data/projects/ac/payload/node_modules/ts-node/dist/index.js:701:20)
    at Object.<anonymous> (/data/projects/ac/payload/src/server.ts:43:33)
    at m._compile (/data/projects/ac/payload/node_modules/ts-node/dist/index.js:708:29)
    at require.extensions.<computed> [as .ts] (/data/projects/ac/payload/node_modules/ts-node/dist/index.js:710:16)
    at main (/data/projects/ac/payload/node_modules/ts-node/dist/bin.js:154:20)
    at Object.<anonymous> (/data/projects/ac/payload/node_modules/ts-node/dist/bin.js:238:5)

DavidOliver avatar Dec 08 '23 17:12 DavidOliver

Hi @cbratschi @DavidOliver this has been fixed and will be out in our next release. Thanks for reporting!

JessChowdhury avatar Dec 13 '23 15:12 JessChowdhury

Hey, I wanna start a new project but I can't because it hasn't been released yet. Is there a way I can start using payload before the relase ?

AhmetHuseyinDOK avatar Dec 19 '23 08:12 AhmetHuseyinDOK

I'd also appreciate a new release. :) Thanks for fixing.

@AhmetHuseyinDOK , you could specify v2.3.1 in your package.json file and npm install?

DavidOliver avatar Dec 19 '23 15:12 DavidOliver

I checked the latest Payload version and there is still an issue left: the getExternalFile() call is silently failing:

https://github.com/payloadcms/payload/blob/8015e999cd5834f532a200ef03fd392d04b3209f/packages/payload/src/uploads/generateFileData.ts#L76

I modified the code to get the thrown exception:

Error [ERR_REQUIRE_ESM]: require() of ES Module /.../node_modules/node-fetch/src/index.js from /.../node_modules/payload/dist/uploads/getExternalFile.js not supported.
Instead change the require of index.js in /.../node_modules/payload/dist/uploads/getExternalFile.js to a dynamic import() which is available in all CommonJS modules.
    at require.extensions.<computed> [as .js] (/.../node_modules/ts-node/dist/index.js:851:20)
    at mod.require (/.../node_modules/next/dist/server/require-hook.js:65:28)
    at /.../node_modules/payload/dist/uploads/getExternalFile.js:56:109
    at async getExternalFile (/U.../node_modules/payload/dist/uploads/getExternalFile.js:56:36) {
  code: 'ERR_REQUIRE_ESM'
}

https://github.com/payloadcms/payload/blob/main/packages/payload/src/uploads/getExternalFile.ts#L15

The transpiled TypeScript code is:

const { default: fetch } = await Promise.resolve().then(()=>/*#__PURE__*/ _interop_require_wildcard(require("node-fetch")));

This is still a require() call and not a dynamic import.

Could you please pass all caught exceptions as cause to a secondary exception: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/cause. In that case no exceptions get lost and it's mich easier to debug such issues.

cbratschi avatar Dec 20 '23 17:12 cbratschi

Could you please re-open this issue? This is still reproducible.

cbratschi avatar Feb 21 '24 15:02 cbratschi

Could you please re-open and patch according to my description below.

Error Handling

First log the error on console like on this line in the same file:

https://github.com/payloadcms/payload/blob/b974a2c042a1b4c58ca9a740d5be7f16bf66177f/packages/payload/src/uploads/generateFileData.ts#L256

Then modify it here:

https://github.com/payloadcms/payload/blob/b974a2c042a1b4c58ca9a740d5be7f16bf66177f/packages/payload/src/uploads/generateFileData.ts#L84

Here is a patch for the transpiled code:

        try {
            if (url && url.startsWith('/') && !disableLocalStorage) {
                const filePath = `${staticPath}/${filename}`;
                const response = await (0, _getFileByPath.default)(filePath);
                file = response;
                overwriteExistingFiles = true;
            } else if (filename && url) {
                file = await (0, _getExternalFile.getExternalFile)({
                    req,
                    data: data
                });
                overwriteExistingFiles = true;
            }
        } catch (err) {
            console.error(err);
            throw new _errors.FileUploadError(req.t);
        }

Now we see all errors.

Better would be to pass the error to FileUploadError:

https://github.com/payloadcms/payload/blob/b974a2c042a1b4c58ca9a740d5be7f16bf66177f/packages/payload/src/errors/FileUploadError.ts#L8

class FileUploadError extends APIError {
  constructor(t?: TFunction, cause: Error) {
    super(
      t ? t('error:problemUploadingFile') : 'There was a problem while uploading the file.',
      httpStatus.BAD_REQUEST,
    )

    this.cause = cause
  }
}

node-fetch Fix

Now we see the root cause of the error:

Error [ERR_REQUIRE_ESM]: require() of ES Module /.../node_modules/node-fetch/src/index.js from /.../node_modules/payload/dist/uploads/getExternalFile.js not supported.
Instead change the require of index.js in /.../node_modules/payload/dist/uploads/getExternalFile.js to a dynamic import() which is available in all CommonJS modules.
    at require.extensions.<computed> [as .js] (/.../node_modules/ts-node/dist/index.js:851:20)
    at mod.require (/.../node_modules/next/dist/server/require-hook.js:65:28)
    at /.../node_modules/payload/dist/uploads/getExternalFile.js:61:109
    at async getExternalFile (/...node_modules/payload/dist/uploads/getExternalFile.js:61:36) {
  code: 'ERR_REQUIRE_ESM'
}

Problem occurs in transpiled code here:

https://github.com/payloadcms/payload/blob/b974a2c042a1b4c58ca9a740d5be7f16bf66177f/packages/payload/src/uploads/getExternalFile.ts#L22

This is the patch for the transpiled code (replace node-fetch by fetch):

const getExternalFile = async ({ data, req })=>{
    const { filename, url } = data;
    if (typeof url === 'string') {
        let fileURL = url;
        if (!url.startsWith('http')) {
            const baseUrl = req.get('origin') || `${req.protocol}://${req.get('host')}`;
            fileURL = `${baseUrl}${url}`;
        }
        //Note: replaced node-fetch by fetch
        //const { default: fetch } = await Promise.resolve().then(()=>/*#__PURE__*/ _interop_require_wildcard(require("node-fetch")));
        const res = await fetch(fileURL, {
            credentials: 'include',
            headers: {
                ...req.headers
            },
            method: 'GET'
        });
        if (!res.ok) throw new _errors.APIError(`Failed to fetch file from ${fileURL}`, res.status);
        //const data = await res.buffer();
        const data = Buffer.from(await res.arrayBuffer());
        return {
            name: filename,
            data,
            mimetype: res.headers.get('content-type') || undefined,
            size: Number(res.headers.get('content-length')) || 0
        };
    }
    throw new _errors.APIError('Invalid file url', 400);
};

Basically these two lines are modified:

//const { default: fetch } = await Promise.resolve().then(()=>/*#__PURE__*/ _interop_require_wildcard(require("node-fetch")));
const data = Buffer.from(await res.arrayBuffer());

cbratschi avatar Apr 18 '24 15:04 cbratschi

@cbratschi Some fixes were pushed in this area. I'd be curious if you still see this issue in 2.14.2.

Regarding replacing node-fetch with fetch. Unfortunately, this is not possible for us to do because Payload v2 supports node versions before 18 which was when the built-in fetch api was introduced.

denolfe avatar Apr 29 '24 14:04 denolfe

@denolfe Still the same. The only change is that the error is now better handled and its message is shown in a toast.

Here is the full error message:

[22:08:46] ERROR (payload): FileRetrievalError: Es gab ein Problem während des Hochladens der Datei. require() of ES Module /.../node_modules/node-fetch/src/index.js from /.../node_modules/payload/dist/uploads/getExternalFile.js not supported.
Instead change the require of index.js in /.../node_modules/payload/dist/uploads/getExternalFile.js to a dynamic import() which is available in all CommonJS modules.
    at generateFileData (/.../node_modules/payload/src/uploads/generateFileData.ts:86:15)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async updateByID (/.../node_modules/payload/src/collections/operations/updateByID.ts:146:57)
    at async updateByIDHandler (/.../node_modules/payload/src/collections/requestHandlers/updateByID.ts:43:17)

The node-fetch ESM module is still being loaded by require(). This would only work with Node's 22 experimental loader.

You could downgrade node-fetch to v2. The TypeScript transpiler converts the async import() to a require() which is not easy to solve.

cbratschi avatar Apr 29 '24 20:04 cbratschi

I am trying to recreate this on [email protected], how can I repro? I have cloned down the public demo repository, I am on node v20.14.0 but when running pnpm generate:types it seems to work fine for me.

JarrodMFlesch avatar Jun 14 '24 13:06 JarrodMFlesch

@JarrodMFlesch the error happens for instance after saving resized images using non-local image storage. In that case Payload uses node-fetch to retrieve the original image data.

This error does not occur while generating Payload types.

A easy test case would be to directly import getFileByURL() and pass any URL. In previous Payload versions the exception was quietly ignored. So double check you get all exceptions.

cbratschi avatar Jun 14 '24 15:06 cbratschi

@JarrodMFlesch the error happens for instance after saving resized images using non-local image storage. In that case Payload uses node-fetch to retrieve the original image data.

This error does not occur while generating Payload types.

A easy test case would be to directly import getFileByURL() and pass any URL. In previous Payload versions the exception was quietly ignored. So double check you get all exceptions.

I ran into this exact error. I am saving my images to the AWS EFS file system, and the initial upload works fine but attempting to save a cropped image throws that node-fetch error.

[17:00:43] ERROR (payload): FileRetrievalError: There was a problem while uploading the file. require() of ES Module /home/node/app/node_modules/node-fetch/src/index.js from /home/node/app/node_modules/payload/dist/uploads/getExternalFile.js not supported.
Instead change the require of index.js in /home/node/app/node_modules/payload/dist/uploads/getExternalFile.js to a dynamic import() which is available in all CommonJS modules.
    at generateFileData (/home/node/app/node_modules/payload/dist/uploads/generateFileData.js:69:23)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async updateByID (/home/node/app/node_modules/payload/dist/collections/operations/updateByID.js:108:61)
    at async updateByIDHandler (/home/node/app/node_modules/payload/dist/collections/requestHandlers/updateByID.js:41:21)

I am on payload 2.18.3, on Node.js 20.10.0.

AshtarCodes avatar Jun 24 '24 17:06 AshtarCodes

We're facing exact same issue using Google Cloud Storage and in local development where we don't have external storage connected.

Crop works fine when uploading an image but after uploading it, it throws the error mentioned above

We're on Payload 2.23.1 and Node.js 20.11.0

sebastianpulak avatar Sep 03 '24 11:09 sebastianpulak