tryretool / retoolrpc

MIT License
6 stars 5 forks source link

SyntaxError: Named export 'RetoolRPC' not found. #11

Closed andreirtaylor closed 9 months ago

andreirtaylor commented 11 months ago

Hello!

Really excited for this project but I'm having a issue.

I am trying to run this as a typescript module and I'm getting some annoying import errors.

Not sure if this is intended to be supported or not at this time, but I put together a repo to reproduce the issue. Let me know if you need anything else.

https://github.com/andreirtaylor/retooltest

jeloi commented 11 months ago

Also running into this issue. Thanks for making the repro @andreirtaylor. Really hope this gets looked at soon as currently this is un-deployable for us.

jeloi commented 11 months ago

@huytool157 @galkin could you please take a look at this? To reiterate - we'd love to use RetoolRPC but are unable to deploy this to production as it's failing at runtime.

I am doing a simple npx tsc for the build, and node --experimental-loader=extensionless dist/index.js to start the server.

import { RetoolRPC } from "retoolrpc";
         ^^^^^^^^^
SyntaxError: Named export 'RetoolRPC' not found. The requested module 'retoolrpc' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'retoolrpc';
const { RetoolRPC } = pkg;

    at ModuleJob._instantiate (node:internal/modules/esm/module_job:122:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:188:5)
    at async CustomizedModuleLoader.import (node:internal/modules/esm/loader:228:24)
    at async loadESM (node:internal/process/esm_loader:40:7)
    at async handleMainPromise (node:internal/modules/run_main:66:12)

Node.js v20.5.1

This is my tsconfig.json:

{
  "include": ["**/*.ts", "**/*.tsx", "**/*.cjs", "./package.json"],
  "exclude": ["node_modules", "dist"],
  "compilerOptions": {
    "allowSyntheticDefaultImports": true,
    "module": "esnext",
    "moduleResolution": "node",
    "target": "ESNext",
    "outDir": "./dist",
    "esModuleInterop": true,
    "strict": true,
    "forceConsistentCasingInFileNames": true,
    "skipLibCheck": true
  }
}

@andreirtaylor has a simple repro using esbuild linked as well. Thank you!

hdoan741 commented 10 months ago

Hi @andreirtaylor and @jeloi: Sorry for the late reply. For Common JS style, can you try this import style instead:

var rt = require('retoolrpc');
const rpc = new rt.RetoolRPC({
  ...
})
hdoan741 commented 10 months ago

There are something wrong when "esbuild" targets "esm" compilation. If you change it to targeting cjs, it works.

See the changes I made here: https://github.com/hdoan741/retooltest/commit/93980ebca2ab0baa7cdd5a78f6a96c716df67bc7

jeloi commented 10 months ago

hey thanks for the response. to clarify, we are not trying to use CommonJS style, we use ESM in Typescript.

We are not able to use require for ESM, so I don't believe this is a suitable fix. I attempted to change the import to using a require and you can see the error:

const rt = require("retoolrpc");
           ^
ReferenceError: require is not defined in ES module scope, you can use import instead
This file is being treated as an ES module because it has a '.js' file extension and 'package.json' contains "type": "module". To treat it as a CommonJS script, rename it to use the '.cjs' file extension.
    at file:///apps/server/dist/retool-rpc/retool-rpc.js:7:12
    at ModuleJob.run (node:internal/modules/esm/module_job:192:25)
    at async CustomizedModuleLoader.import (node:internal/modules/esm/loader:228:24)
    at async loadESM (node:internal/process/esm_loader:40:7)
    at async handleMainPromise (node:internal/modules/run_main:66:12)
hdoan741 commented 10 months ago

@jeloi Yeah, sorry. I took a look at the repo created by @andreirtaylor, and I think I fixed it by using --format=cjs on the esbuild command. See my comment above & the commit I created to fix the issue.

jeloi commented 10 months ago

We use tsc not esbuild for our build pipeline (see above tsconfig.json), so that change won't work for us unfortunately. Moreover, I don't think it would be feasible or desirable to change our build resolution method just to accommodate the retool package. Can this be resolved such that the package can support ESM?

hdoan741 commented 10 months ago

We do intend to support it. We build the esm module here:

https://github.com/tryretool/retoolrpc/blob/main/javascript/package.json#L15-L30

When I try the import to

import * as retool from 'retoolrpc'
const rpc = new RetoolRPC({
...
})

It does seem to pass the error above. Can you try that and let me know if it works or you see any other error?

jeloi commented 10 months ago

That's leading to this error now:

(node:1106) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.
/node_modules/.pnpm/retoolrpc@0.1.1/node_modules/retoolrpc/dist/index.js:1
export { R as RetoolRPC } from './rpc-aa760f24.js';
^^^^^^

SyntaxError: Unexpected token 'export'
    at internalCompileFunction (node:internal/vm:73:18)
    at wrapSafe (node:internal/modules/cjs/loader:1153:20)
    at Module._compile (node:internal/modules/cjs/loader:1197:27)
    at Module._extensions..js (node:internal/modules/cjs/loader:1287:10)
    at Module.load (node:internal/modules/cjs/loader:1091:32)
    at Module._load (node:internal/modules/cjs/loader:938:12)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:165:29)
    at ModuleJob.run (node:internal/modules/esm/module_job:192:25)
    at async CustomizedModuleLoader.import (node:internal/modules/esm/loader:228:24)
    at async loadESM (node:internal/process/esm_loader:40:7)
hdoan741 commented 10 months ago

I think it is a node issue with how it detects the module type. We'll need to create a new release with explicit .mjs files for esm.

I've verified that it works with the test repo above here: https://github.com/hdoan741/retooltest/commit/1b361c0bfc42fea49467edc9d5f693344cd23b12

You can try it out by renaming the following files:

mv node_modules/retoolrpc/dist/rpc-aa760f24.js node_modules/retoolrpc/dist/rpc-aa760f24.mjs
mv node_modules/retoolrpc/dist/index.js node_modules/retoolrpc/dist/index.mjs

then change the dist/index.js to dist/index.esm innode_modules/retoolrpc/package.json`

      "import": "./dist/index.mjs",

and the import in node_modules/retoolrpc/dist/index.mjs to this

export { R as RetoolRPC } from './rpc-aa760f24.mjs';

Anyway, will look into releasing a version with the fix ASAP.

hdoan741 commented 10 months ago

@jeloi We've published a new version that should build mjs files for ESM module. Can you try again? It should be v0.1.3 for javascript

jeloi commented 9 months ago

Worked - thank you!