-
Notifications
You must be signed in to change notification settings - Fork 3
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
Import trace for requested module #1
Comments
Hey @ahmedrowaihi! 👋🏻 We need more information to help you. |
Hey @RomainLanz git clone https://github.com/ahmedrowaihi/flydrive-trace-next-app.git pnpm install pnpm dev reproduce:
|
I had no issue locally but I have:
|
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 ? |
Logs shows in next console not browser tho |
Alright, gotcha. The code works fine; the issue is the log sent to the console. |
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 something needs to be refactored, I will try to get back next week and refactor this after I get my midterm exams done |
I will also check on my side to see if anything come to my mind. Good luck with your exams! 🙌🏻 |
Just starting trying flydrive this weekend. Same error in NextJS + PNPM + Turborepo |
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 |
It is a known issue of Webpack. @Julien-R44 already made some investigation in Bentocache for the same issue. |
@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
patch file: 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, |
Uhmm, would be the long term solution here? I understand poppings do that to have sub-set of lodash functionality no? |
@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 best solution is that we may reallocate the export of fsImportAll and importDefault to their own path, ( not in index ) |
Yes, this would be ideal |
the error occurs every time flydrive-js uses an import from which hits the @poppings/utils/index.ts and causes the error |
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 |
How would that work? How updating other AdonisJS packages being updated can be avoided? |
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' |
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 |
no, they don't have to, both imports are valid import { slash } from '@poppinss/utils' and using the second will avoid the issue for nextjs, without breaking the lib for other libs |
Ahh I got it 👌 Great. Can you do the PR? |
Sure, only if @RomainLanz agrees on the solution suggested |
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.. So the code on the two methods above is not supported in webpack |
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 |
Let me know if you need help, I already suggested some fixes above |
Yeah. I think we will go with the submodules approach (as you suggested) in |
I keep getting this error in console
I am in a monorepo using PNPM & Turbo
The text was updated successfully, but these errors were encountered: