CJS issue for stream-mime-type

jhoopes commented 4 months ago

Hello,

Apologies if I'm just not following differences between CJS and ESM and i just need to try to change my usage to ESM, and if i'm wrong please correct me, but it appears that the dependency for the stream-mime-type of file-type does not support CJS, and thus the build for that package here also doesn't work.

on the built CJS dist file for stream-mime-type file in the resolveMimeType function, const { fileTypeFromBuffer } = await import('file-type')); gets converted into: const { fileTypeFromBuffer } = await Promise.resolve().then(() => require('file-type')); But, when running this trying to write a file to S3, it errors out on me with:

UnableToWriteFile [Error]: Unable to write the file. Reason: require() of ES Module /usr/src/app/node_modules/@flystorage/stream-mime-type/node_modules/file-type/index.js from /usr/src/app/node_modules/@flystorage/stream-mime-type/dist/cjs/stream-mime-type.js not supported.
Instead change the require of index.js in /usr/src/app/node_modules/@flystorage/stream-mime-type/dist/cjs/stream-mime-type.js to a dynamic import() which is available in all CommonJS modules.
at UnableToWriteFile.because \u001b[90m(/usr/src/app/\u001b[39mnode_modules/\u001b[4m@flystorage\u001b[24m/file-storage/dist/cjs/errors.js:55:71\u001b[90m)\u001b[39m\r\n    at FileStorage.write \u001b[90m(/usr/src/app/\u001b[39mnode_modules/\u001b[4m@flystorage\u001b[24m/file-storage/dist/cjs/file-storage.js:84:44

when I update the built cjs file locally to use the dynamic import, it does work for my use-case. Could this line: https://github.com/duna-oss/flystorage/blob/fd191534a156627f8ef2ad34ac112f0a76e40938/packages/stream-mime-type/src/stream-mime-type.ts#L55. be extracted or forced to be an import for the CJS build? Or something else? Will try to dig around more, just didn't know if anyone else had ideas to try to make the CJS build work.

frankdejonge commented 3 months ago

Thanks for reporting, I'll check this out, see what I can do. Might have to do with a changed output setting for tsc, but I'm not sure. Will report back.

frankdejonge commented 3 months ago

@jhoopes just for my info, on what runtime+version are you running this?

frankdejonge commented 3 months ago

I think I found the issue, I was using node (an alias for node10), which is outdated. I'll convert the CJS output to node16, which supports dynamic imports for CJS.

frankdejonge commented 3 months ago

After some struggles, the latest version should resolve your CJS issue. Can you verify and report back?

jhoopes commented 3 months ago

hey @frankdejonge , thanks so much for jumping on this.

Yeah I was trying to make a slightly different 'hacky' way to get working that I hade seen others make on similar issues in order to make the change less large.

const dynamicImport = new Function('module', 'return import(module)');
const {fileTypeFromBuffer} = await dynamicImport('file-type');

which had fixed the issue in the built files, but the tests were failing and hadn't yet dug into more of why as the ESM build of this kept saying it couldn't find a module in the test. Maybe that was all of the other changes you had made to the build scripts.

But this is fixed with the most recent updates. Thanks again for jumping on to help figure this out.

In case you were curious or needed, I was running the LTS node version in a built docker image/container:

FROM node:lts-alpine

ARG USERID=1000
ARG GROUPID=1000

RUN apk add --no-cache bash shadow
RUN apk add dumb-init

RUN groupmod -g $GROUPID node && usermod -u $USERID -g $GROUPID node

ENV NODE_ENV production
WORKDIR /usr/src/app
COPY --chown=node:node . .
RUN npm ci --only=production

USER node
CMD [ "dumb-init", "node", "build/index.js" ]

frankdejonge commented 3 months ago

Great to hear, I'll mark this one as fixed then 👍

duna-oss / flystorage

CJS issue for stream-mime-type #45