payloadcms / payload

Payload is the open-source, fullstack Next.js framework, giving you instant backend superpowers. Get a full TypeScript backend and admin panel instantly. Use Payload as a headless CMS or for building powerful applications.
https://payloadcms.com
MIT License
23.24k stars 1.46k forks source link

node.fetch ESM import issue in getFileByURL.js #4421

Open cbratschi opened 9 months ago

cbratschi commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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

cbratschi commented 9 months ago

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 commented 9 months ago

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

denolfe commented 9 months ago

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

DavidOliver commented 9 months ago

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)
JessChowdhury commented 9 months ago

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

AhmetHuseyinDOK commented 8 months ago

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 ?

DavidOliver commented 8 months ago

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?

cbratschi commented 8 months ago

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 commented 6 months ago

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

cbratschi commented 4 months ago

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());
denolfe commented 4 months ago

@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.

cbratschi commented 4 months ago

@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.

JarrodMFlesch commented 3 months ago

I am trying to recreate this on payload@2.20.0, 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.

cbratschi commented 3 months ago

@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.

AshtarCodes commented 2 months ago

@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.

sebastianpulak commented 1 week ago

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