-
-
Notifications
You must be signed in to change notification settings - Fork 114
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
Convert worker to ESM #500
Conversation
Putting in draft to fix linter errors:
|
Commit message from 98e899c
|
One downside of using |
That's weird. How does Next not have to do that? Some kind of webpack thing? |
I'll check. Maybe we can adopt a similar solution. Currently, the worker at least is not able to import stuff without the
The next app runs fine without the extension |
Somewhat related discussion: nodejs/node#46006 But I just noticed the worker code doesn't run without
Putting this back into draft. |
I found out that they use this by printing the default webpack config: webpack: (config, { isServer, dev }) => {
+ console.log(config.module.rules)
if (isServer) {
|
Trying to make Finally found a ticket related to the problem I run into then: vercel/next.js#34412 But no solution there... afaict, it should be fixed but it still happens for me. But I think the solution is related to But when I use diff --git a/.babelrc b/.babelrc
index e5ef908..fbe2a18 100644
--- a/.babelrc
+++ b/.babelrc
@@ -15,6 +15,12 @@
]
}
}
+ ],
+ [
+ "@babel/plugin-transform-modules-commonjs",
+ {
+ "importInterop": "babel"
+ }
]
]
}
\ No newline at end of file These errors stop:
but I get new errors now:
🙄 Then I tried to use a function for
So I used
... (just trying to write down my process so I don't start to run in circles, lol) UPDATE: And using
this but then I get hit by
So I think everything is broken now, lol |
the this is why it's been commonjs for so long. I don't tend to do configuration programming very well |
No, it's not about this at the moment. I realized that there is a conflict between app and worker: the app doesn't work with setting
which I believe is related to But it's required to allow imports from the worker since else, it assumes the imports to be from CommonJS:
but I can just try to use what is suggested in the error instead of trying to make ESM imports work:
I guess it's more specifically related to Javascript configuration, haha There are so many layers: babel, webpack, ESM, CJS and then NextJS on top which tries to handle all of them at the same time |
Nah, that also doesn't work ... the worker then doesn't understand
So we're back to square one. Can't import ESM stuff from the app in the worker without using But then this CJS creeps more and more into the app codebase, which I wanted to finally avoid UPDATE: Oh, and I wanted to import
|
Okay, I just created a package.json with This doesn't break the app but fixes the worker. I also noticed that we used |
To use ESM for the worker, I created a package.json file in worker/ with `{ type: "module" }` as its sole content. I then rewrote every import to use ESM syntax. I also tried to set `{ type: "module" }` in the root package.json file to also use ESM in next.config.js. However, this resulted in a weird problem: default imports were now getting imported as objects in this shape: `{ default: <defaultImport> }`. Afaik, this should only be the case if you use "import * as foo from 'bar'" syntax: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#default_import This is fixed by not using `{ type: "module" }` for some reason. However, then, next.config.js also doesn't support ESM import syntax anymore. The documentation says that if you want to use ESM, you can use next.config.mjs: https://nextjs.org/docs/pages/api-reference/next-config-js But I didn't want to use MJS extension since I don't have any experience with it. For example, not sure if it's good style to mix JS with MJS etc. So I kept the CJS import syntax there.
I wasn't able to fix the following errors: /home/runner/work/stacker.news/stacker.news/worker/auction.js:0:0: Parsing error: No Babel config file detected for /home/runner/work/stacker.news/stacker.news/worker/auction.js. Either disable config file checking with requireConfigFile: false, or configure Babel so that it can find the config files. (null) /home/runner/work/stacker.news/stacker.news/worker/earn.js:0:0: Parsing error: No Babel config file detected for /home/runner/work/stacker.news/stacker.news/worker/earn.js. Either disable config file checking with requireConfigFile: false, or configure Babel so that it can find the config files. (null) /home/runner/work/stacker.news/stacker.news/worker/index.js:0:0: Parsing error: No Babel config file detected for /home/runner/work/stacker.news/stacker.news/worker/index.js. Either disable config file checking with requireConfigFile: false, or configure Babel so that it can find the config files. (null) /home/runner/work/stacker.news/stacker.news/worker/nostr.js:0:0: Parsing error: No Babel config file detected for /home/runner/work/stacker.news/stacker.news/worker/nostr.js. Either disable config file checking with requireConfigFile: false, or configure Babel so that it can find the config files. (null) /home/runner/work/stacker.news/stacker.news/worker/ots.js:0:0: Parsing error: No Babel config file detected for /home/runner/work/stacker.news/stacker.news/worker/ots.js. Either disable config file checking with requireConfigFile: false, or configure Babel so that it can find the config files. (null) /home/runner/work/stacker.news/stacker.news/worker/repin.js:0:0: Parsing error: No Babel config file detected for /home/runner/work/stacker.news/stacker.news/worker/repin.js. Either disable config file checking with requireConfigFile: false, or configure Babel so that it can find the config files. (null) /home/runner/work/stacker.news/stacker.news/worker/search.js:0:0: Parsing error: No Babel config file detected for /home/runner/work/stacker.news/stacker.news/worker/search.js. Either disable config file checking with requireConfigFile: false, or configure Babel so that it can find the config files. (null) /home/runner/work/stacker.news/stacker.news/worker/streak.js:0:0: Parsing error: No Babel config file detected for /home/runner/work/stacker.news/stacker.news/worker/streak.js. Either disable config file checking with requireConfigFile: false, or configure Babel so that it can find the config files. (null) /home/runner/work/stacker.news/stacker.news/worker/trust.js:0:0: Parsing error: No Babel config file detected for /home/runner/work/stacker.news/stacker.news/worker/trust.js. Either disable config file checking with requireConfigFile: false, or configure Babel so that it can find the config files. (null) /home/runner/work/stacker.news/stacker.news/worker/views.js:0:0: Parsing error: No Babel config file detected for /home/runner/work/stacker.news/stacker.news/worker/views.js. Either disable config file checking with requireConfigFile: false, or configure Babel so that it can find the config files. (null) /home/runner/work/stacker.news/stacker.news/worker/wallet.js:0:0: Parsing error: No Babel config file detected for /home/runner/work/stacker.news/stacker.news/worker/wallet.js. Either disable config file checking with requireConfigFile: false, or configure Babel so that it can find the config files. (null) I tried to tell babel where to find the babel config file (.babelrc), specifying the babel config in worker/package.json under "babel", using babel.config.json etc. to no avail. However, afaict, we don't need babel for the worker since it won't run in a browser. Babel is only used to transpile code to target browsers. But it still would be nice to lint the worker code with standard. But we can figure this out later.
This fixes the issue that we can't use `{ "type": "module" }` in the root package.json since it breaks the app with this error: app | TypeError: next_auth_providers_credentials__WEBPACK_IMPORTED_MODULE_2__ is not a function app | at eval (webpack-internal:///./pages/api/auth/[...nextauth].js:218:20) app | at process.processTicksAndRejections (node:internal/process/task_queues:95:5) app | LND GRPC connection successful app | - error pages/api/auth/[...nextauth].js (139:2) @ CredentialsProvider app | - error Error [TypeError]: next_auth_providers_credentials__WEBPACK_IMPORTED_MODULE_2__ is not a function app | at eval (webpack-internal:///./pages/api/auth/[...nextauth].js:218:20) { app | digest: undefined app | } app | 137 | app | 138 | const providers = [ app | > 139 | CredentialsProvider({ app | | ^ app | 140 | id: 'lightning', app | 141 | name: 'Lightning', app | 142 | credentials: { app | TypeError: next_auth_providers_credentials__WEBPACK_IMPORTED_MODULE_2__ is not a function app | at eval (webpack-internal:///./pages/api/auth/[...nextauth].js:218:20) app | at process.processTicksAndRejections (node:internal/process/task_queues:95:5) build but we need to tell the worker that the files are MJS, else we get this error: worker | file:///app/worker/wallet.js:3 worker | import { datePivot } from '../lib/time.js' worker | ^^^^^^^^^ worker | SyntaxError: Named export 'datePivot' not found. The requested module '../lib/time.js' is a CommonJS module, which may not support all module.exports as named exports. worker | CommonJS modules can always be imported via the default export, for example using: worker | worker | import pkg from '../lib/time.js'; worker | const { datePivot } = pkg; worker | worker | at ModuleJob._instantiate (node:internal/modules/esm/module_job:124:21) worker | at async ModuleJob.run (node:internal/modules/esm/module_job:190:5) worker | worker | Node.js v18.17.0 worker | worker exited with code 1
I did not want to do this but I was not able to fix this error in any other way I tried: /home/ekzyis/programming/stacker.news/api/lnd/index.js:0:0: Parsing error: No Babel config file detected for /home/ekzyis/programming/stacker.news/api/lnd/index.js. Either disable config file checking with requ ireConfigFile: false, or configure Babel so that it can find the config files. (null) Did not want to look deeper into all this standard, eslint, babel configuration stuff ...
To use ESM for the worker, I created a package.json file in worker/ with
{ type: "module" }
as its sole content.I then rewrote every import to use ESM syntax.
I also tried to set
{ type: "module" }
in the root package.json file to also use ESM in next.config.js.However, this resulted in a weird problem: default imports were now getting imported as objects in this shape:
{ default: <defaultImport> }
.Afaik, this should only be the case if you use "import * as foo from 'bar'" syntax: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#default_import
This is fixed by not using
{ type: "module" }
for some reason. However, then, next.config.js also doesn't support ESM import syntax anymore.The documentation says that if you want to use ESM, you can use next.config.mjs: https://nextjs.org/docs/pages/api-reference/next-config-js
But I didn't want to use MJS extension since I don't have any experience with it. For example, not sure if it's good style to mix JS with MJS etc. So I kept the CJS import syntax there.