Skip to content
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.

Sign up for GitHub

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

Jump to bottom

bug: fixed bug when trying to import module using node esm and added … #190

Closed
wants to merge 1 commit into from
Closed

bug: fixed bug when trying to import module using node esm and added … #190

wants to merge 1 commit into from

Conversation

twilson63
Copy link
Contributor

…documentation to use ESM web bundles

Removed exports property from package.json, this property blocks the ability to consume the NodeJS ESM library using nodeJS ESM projects.

Also, I added some documentation on how to reference web-based ESM bundles.

ppedziwiatr
ppedziwiatr reviewed Jul 12, 2022
View reviewed changes
package.json
"require": "./lib/cjs/index.js"
},
"./esm": "bundles/esm.bundle.js",
"./web": "bundles/web.bundle.js"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this not break the #152 (comment) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To reference the bundled web and esm you would use

import { WarpWebFactory } from 'warp-contracts/bundles/esm.bundle'

instead of

import { WarpWebFactory } from 'warp-contracts/esm'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I continue to struggle with this issue, but I have narrowed down the root cause. The ./lib/esm/index.js build is not esm compliant, I don't know why it is not, but I do know when I switch the exports config to point the import to cjs it works fine.

"exports": {
    ".": {
      "import": "./lib/cjs/index.js",
      "require": "./lib/cjs/index.js"
    },
    "./esm": "bundles/esm.bundle.js",
    "./web": "bundles/web.bundle.js"
  },

So that may be another option.

The current esm build results in this response:

file:///workspace/stamp-coin/copy-and-deploy.mjs:4
import { WarpFactory } from './warp/lib/esm/index.js'
         ^^^^^^^^^^^
SyntaxError: Named export 'WarpFactory' not found. The requested module './warp/lib/esm/index.js' 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 './warp/lib/esm/index.js';
const { WarpFactory } = pkg;

For now, I am just pulling down the latest source and patching the package.json

npx degit warp-contracts/warp#v1.2.0-rc.1 warp
cd warp
npx json -I -f package.json -e 'this.exports.import = "./lib/cjs/index.js"'
yarn && yarn build:cjs

Please, let me know how I can help move this bug to fixed.

@ppedziwiatr
Copy link
Contributor

I believe that to make it compatible with .mjs files, we would need to add "type": "module" in the SDK's package.json...but this will probably break some other stuff :-)
The ESM version is built by the typescript compiler (https://github.com/warp-contracts/warp/blob/main/tsconfig.esm.json), so I really don't understand how can it be not compatible with esm module format :-)

We will return to this next week.

@asiaziola
Copy link
Contributor

@twilson63 @ppedziwiatr I was able to make the .mjs file run correctly only using tsup npm package (https://github.com/egoist/tsup), looks cool and is based on esbuild but I'm not sure what's happening underneath. It requires a lot more tests but maybe it's a good start.

@ppedziwiatr
Copy link
Contributor

ppedziwiatr commented Aug 21, 2022 via email

@ppedziwiatr
Copy link
Contributor

ppedziwiatr commented Aug 21, 2022 via email

@ppedziwiatr
Copy link
Contributor

ppedziwiatr commented Aug 21, 2022 via email

@ppedziwiatr
Copy link
Contributor

ppedziwiatr commented Aug 21, 2022 via email

@ppedziwiatr
Copy link
Contributor

ppedziwiatr commented Aug 21, 2022 via email

@asiaziola
Copy link
Contributor

asiaziola commented Aug 21, 2022 •
edited

Yeah, I was trying this one. I got 'Permission denied' when running sh script so I dropped the idea 🤡 but as I see simply running

chmod +x build.sh

should make it work. I'll try again and let you know.

@asiaziola
Copy link
Contributor

Weird af, .mjs works fine with cjs version, with esm I'm getting

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/Users/asiaziola/arweave/redstone-smartcontracts/lib/esm/logging/index' imported from /Users/asiaziola/arweave/redstone-smartcontracts/lib/esm/index.js

@asiaziola
Copy link
Contributor

Seems like this is a cause https://bobbyhadz.com/blog/javascript-error-err-module-not-found-cannot-find-module#:~:text=The%20%22Error%20%5BERR_MODULE_NOT_FOUND%5D%3A,extension%20when%20importing%20local%20files.

@asiaziola
Copy link
Contributor

so I guess we'd need extensions for all the imported files?

@ppedziwiatr
Copy link
Contributor

ppedziwiatr commented Aug 21, 2022 via email

@ppedziwiatr
Copy link
Contributor

ppedziwiatr commented Aug 21, 2022 via email

@asiaziola
Copy link
Contributor

I dont think we should compare these libs... they have one-file package. The problem is not what is written in package.json (we have it compatible with them), I believe the problem is how we import classes etc.

@asiaziola
Copy link
Contributor

asiaziola commented Aug 21, 2022 •
edited

I'll play with it some more :))))

@ppedziwiatr ppedziwiatr mentioned this pull request Aug 22, 2022
Closed
@ppedziwiatr
Copy link
Contributor

@asiaziola , we should probably store somewhere (probably here #209 ?) all the related articles, SO hints node and web bundlers docs/issues - that will document all the tricks that we're currently using to make all the versions work...

@ppedziwiatr
Copy link
Contributor

I believe this has been finally fixed in 1.2.0-rc.2, https://github.com/warp-contracts/bundlers#readme

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ppedziwiatr ppedziwiatr ppedziwiatr left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@twilson63 @ppedziwiatr @asiaziola