Import trace for requested module

[ahmedrowaihi]

I keep getting this error in console I am in a monorepo using PNPM & Turbo

 ⚠ ../../node_modules/.pnpm/@poppinss+utils@6.7.3/node_modules/@poppinss/utils/build/index.js
Critical dependency: the request of a dependency is an expression

Import trace for requested module:
../../node_modules/.pnpm/@poppinss+utils@6.7.3/node_modules/@poppinss/utils/build/index.js
../../node_modules/.pnpm/flydrive@1.0.2_@aws-sdk+client-s3@3.600.0_@aws-sdk+s3-request-presigner@3.600.0/node_modules/flydrive/build/chunk-XTE5URPJ.js
../../node_modules/.pnpm/flydrive@1.0.2_@aws-sdk+client-s3@3.600.0_@aws-sdk+s3-request-presigner@3.600.0/node_modules/flydrive/build/drivers/s3/main.js

[RomainLanz]

Hey @ahmedrowaihi! 👋🏻

We need more information to help you. The best would be to have a repository with the minimum amount of code to reproduce your issue.

[ahmedrowaihi]

Hey @RomainLanz made one for you flydrive-trace-next-app

git clone https://github.com/ahmedrowaihi/flydrive-trace-next-app.git

pnpm install

pnpm dev

reproduce:

• visit home page: http://localhost:3000/
• upload image fs or s3
• click upload
• watch console logs

[RomainLanz]

I had no issue locally but I have:

• Remove the custom endpoint (since I don't have any)
• Added the region to the .env file
• Remove the URL generation, since it is not needed too

[ahmedrowaihi]

the S3 driver provided is just an example, you can ignore it or change it work with your s3,

the issue in the log appears for all of the drivers due to some @poppinss/utils imports

Object is getting uploaded successfully,but the logs shows this warning all the time..

Have you tried using pnpm ? Try clean node_modules, install with pnpm, and check the reproduce steps above

[ahmedrowaihi]

Logs shows in next console not browser tho

[RomainLanz]

Alright, gotcha.

The code works fine; the issue is the log sent to the console. I am not sure where that comes from. It is probably Turbo/Webpack.

[ahmedrowaihi]

it's probably webpack, or the dist of flydrive something is conflicting

The repo I gave you above doesn't use turbo, it shows same error also running with npm, showed the same..

something needs to be refactored, I will try to get back next week and refactor this after I get my midterm exams done

[RomainLanz]

I will also check on my side to see if anything come to my mind.

Good luck with your exams! 🙌🏻

[andresgutgon]

Just starting trying flydrive this weekend. Same error in NextJS + PNPM + Turborepo

[andresgutgon]

Could be this https://github.com/poppinss/utils/blob/develop/src/import_default.ts#L19 ?

I'm reading that Webpack can't require dynamically https://stackoverflow.com/a/59235546

I've no idea tbh. Please don't get confused for my guess

[ahmedrowaihi]

Could be this https://github.com/poppinss/utils/blob/develop/src/import_default.ts#L19 ?

I'm reading that Webpack can't require dynamically https://stackoverflow.com/a/59235546

I've no idea tbh. Please don't get confused for my guess

hmm, I will try to eject only needed utilities for flydrive to work without using Poppins and test it again

[RomainLanz]

It is a known issue of Webpack. @Julien-R44 already made some investigation in Bentocache for the same issue.

[ahmedrowaihi]

@andresgutgon you are right, the issue was due to the dynamic require

you can use this patch i made to get rid of the issue currently, the removed methods doesn't affect flydrive, it's not used on it these methods are removed in the patch..

• fsImportAll

• importDefault

  patch file: @poppinss+utils+6.7.3.patch you can use it with patch-package

diff --git a/node_modules/@poppinss/utils/build/index.js b/node_modules/@poppinss/utils/build/index.js
index 84ca468..10ba893 100644
--- a/node_modules/@poppinss/utils/build/index.js
+++ b/node_modules/@poppinss/utils/build/index.js
@@ -111,19 +111,6 @@ var RuntimeException = class extends Exception {
   static status = 500;
 };

-// src/import_default.ts
-async function importDefault(importFn, filePath) {
-  const moduleExports = await importFn();
-  if (!("default" in moduleExports)) {
-    const errorMessage = filePath ? `Missing "export default" in module "${filePath}"` : `Missing "export default" from lazy import "${importFn}"`;
-    throw new RuntimeException(errorMessage, {
-      cause: {
-        source: importFn
-      }
-    });
-  }
-  return moduleExports.default;
-}

 // src/define_static_property.ts
 import lodash from "@poppinss/utils/lodash";
@@ -242,30 +229,6 @@ function isScriptFile(filePath) {
   return false;
 }

-// src/fs_import_all.ts
-async function importFile(basePath, fileURL, values, options) {
-  const filePath = fileURLToPath2(fileURL);
-  const fileExtension = extname2(filePath);
-  const collectionKey = relative(basePath, filePath).replace(new RegExp(`${fileExtension}$`), "").split(sep);
-  const exportedValue = fileExtension === ".json" ? await import(fileURL, { assert: { type: "json" } }) : await import(fileURL);
-  lodash2.set(
-    values,
-    options.transformKeys ? options.transformKeys(collectionKey) : collectionKey,
-    exportedValue.default ? exportedValue.default : { ...exportedValue }
-  );
-}
-async function fsImportAll(location, options) {
-  options = options || {};
-  const collection = {};
-  const normalizedLocation = typeof location === "string" ? location : fileURLToPath2(location);
-  const files = await fsReadAll(normalizedLocation, {
-    filter: isScriptFile,
-    ...options,
-    pathType: "url"
-  });
-  await Promise.all(files.map((file) => importFile(normalizedLocation, file, collection, options)));
-  return collection;
-}

 // src/message_builder.ts
 var MessageBuilder = class {
@@ -406,11 +369,9 @@ export {
   createError,
   defineStaticProperty,
   flatten,
-  fsImportAll,
   fsReadAll,
   getDirname,
   getFilename,
-  importDefault,
   isScriptFile,
   joinToURL,
   naturalSort,

[andresgutgon]

Uhmm, would be the long term solution here?

I understand poppings do that to have sub-set of lodash functionality no?

[ahmedrowaihi]

@poppings/utils is a utility used for all adonisjs libs/frameworks

in case of flydrive, it uses few utils for runtime exception and string/byte matching etc...

the thing is that @poppings/utils exports most functions at the index.ts

using my patch above won't hurt if you use any adonisjs lib like: flydrive with nextjs unless the lib needs fsImportAll or importDefault functions, which won't work with nextjs

best solution is that we may reallocate the export of fsImportAll and importDefault to their own path, ( not in index ) so only libs that need them, can import them through @poppings/utils/import-helpers

[andresgutgon]

best solution is that we may reallocate the export of fsImportAll and importDefault to their own path, ( not in index ) so only libs that need them, can import them through @poppings/utils/import-helpers

Yes, this would be ideal

[ahmedrowaihi]

the error occurs every time flydrive-js uses an import from @poppings/utils <- see this search result

which hits the @poppings/utils/index.ts and causes the error

[ahmedrowaihi]

best solution is that we may reallocate the export of fsImportAll and importDefault to their own path, ( not in index ) so only libs that need them, can import them through @poppings/utils/import-helpers

Yes, this would be ideal

it might be good, unless other adonis libs expects those helpers from index.ts, then all those lib needs to be updated

maybe we can extend exports from package.json instead :'D

[andresgutgon]

maybe we can extend exports from package.json instead :'D

How would that work? How updating other AdonisJS packages being updated can be avoided?

[ahmedrowaihi]

applying these changes to @poppinss/utils

diff --git a/package.json b/package.json
index 446b6f7..2fd4f76 100644
--- a/package.json
+++ b/package.json
@@ -22,7 +22,10 @@
     "./string": "./build/src/string/main.js",
     "./string_builder": "./build/src/string_builder.js",
     "./json": "./build/src/json/main.js",
-    "./types": "./build/src/types.js"
+    "./types": "./build/src/types.js",
+    "./exceptions": "./build/exceptions/src/exceptions/index.js",
+    "./exception": "./build/src/exception.js",
+    "./slash": "./build/src/slash.js"
   },
   "engines": {
     "node": ">=18.16.0"
@@ -35,7 +38,7 @@
     "clean": "del-cli build",
     "typecheck": "tsc --noEmit",
     "precompile": "npm run lint && npm run clean",
-    "compile": "tsup-node && tsc --emitDeclarationOnly --declaration",
+    "compile": "tsup-node && tsc --declaration",
     "build": "npm run compile && npm run build:lodash",
     "release": "np",
     "version": "npm run build",
diff --git a/src/exceptions/index.ts b/src/exceptions/index.ts
new file mode 100644
index 0000000..138f86b
--- /dev/null
+++ b/src/exceptions/index.ts
@@ -0,0 +1,2 @@
+export * from './invalid_arguments_exception.js'
+export * from './runtime_exception.js'

will allow us to modify flydrive only without affecting other libs

for example this could be changed to:

-import { slash } from '@poppinss/utils'
+import { slash } from '@poppinss/utils/slash'

[andresgutgon]

But consumers of this package still needs to be updated no?

As a nextjs user I think is worthy that the package works without errors out of the box. But let's see what others say

[ahmedrowaihi]

no, they don't have to, because this added more ways to imports, it doesn't change existing imports by adding exports to package.json

both imports are valid

import { slash } from '@poppinss/utils' import { slash } from '@poppinss/utils/slash'

and using the second will avoid the issue for nextjs, without breaking the lib for other libs

[andresgutgon]

Ahh I got it 👌 Great. Can you do the PR?

[ahmedrowaihi]

Ahh I got it 👌 Great. Can you do the PR?

Sure, only if @RomainLanz agrees on the solution suggested

[thetutlage]

Can I understand the root of the problem? Why does TurboPack complains about a function that performs a dynamic import or a filesystem read operation?

Sorry, I am not aware of Turbopack or internal workings of Next.js, so I am trying to understand what is the root of the warning.

[ahmedrowaihi]

Can I understand the root of the problem? Why does TurboPack complains about a function that performs a dynamic import or a filesystem read operation?

Sorry, I am not aware of Turbopack or internal workings of Next.js, so I am trying to understand what is the root of the warning.

Turbopack has no problems with dynamic imports.. Webpack does, because it do static analysis for every import statement at bundle time..

So the code on the two methods above is not supported in webpack

[thetutlage]

Okay. In that case, I am not sure if FlyDrive is meant to be used with WebPack or any bundler for that matter.

I don't want to discourage the usage of FlyDrive, but if we end up in a cycle where we have to optimize/re-structure code to make FlyDrive work with a bundler, then I might not be eager to do so. Simply, because I want to keep my projects simple and easy to maintain for the longer run.

Now, regarding this simple change of @poppinss/utils, I will go ahead and provide a fix for it and see if everything goes smoothly beyond that. 👍

[ahmedrowaihi]

Okay. In that case, I am not sure if FlyDrive is meant to be used with WebPack or any bundler for that matter.

I don't want to discourage the usage of FlyDrive, but if we end up in a cycle where we have to optimize/re-structure code to make FlyDrive work with a bundler, then I might not be eager to do so. Simply, because I want to keep my projects simple and easy to maintain for the longer run.

Now, regarding this simple change of @poppinss/utils, I will go ahead and provide a fix for it and see if everything goes smoothly beyond that. 👍

Let me know if you need help, I already suggested some fixes above

[thetutlage]

Yeah. I think we will go with the submodules approach (as you suggested) in @poppinss/utils.

[ahmedrowaihi]

Should we close this ?:)

[andresgutgon]

Is poppins version bumped in this project with the fix?