ethereumjs-monorepo icon indicating copy to clipboard operation
ethereumjs-monorepo copied to clipboard

Build errors with newest ethereumjs/evm

Open thevaizman opened this issue 3 years ago β€’ 6 comments
trafficstars

Hey @jochem-brouwer @holgerd77,

I encountered 2 build errors using the newest package of @ethereumjs/evm (1.0.0) that prevent me from deploying my evm.codes app to Vercel. While I am able to easily solve these errors locally (by changing the sources of the evm package, this is obviously not ideal and I would like to know what is the correct way to handle these errors so that I can deploy to Vercel using the original package.

The first error:

./node_modules/@ethereumjs/evm/src/evm.ts:12:1
Type error: Import assignment cannot be used when targeting ECMAScript modules. Consider using 'import * as ns from "mod"', 'import {a} from "mod"', 'import d from "mod"', or another module format instead.

  10 |   zeros,
  11 | } from '@ethereumjs/util'
> 12 | import AsyncEventEmitter = require('async-eventemitter')
     | ^
  13 | import { debug as createDebugLogger } from 'debug'
  14 | import { promisify } from 'util'
  15 | 
error Command failed with exit code 1.

I am able to solve this easily by changing the erroneous line to import AsyncEventEmitter from 'async-eventemitter'

The second error:

./node_modules/@ethereumjs/evm/src/precompiles/index.ts:204:3
Type error: Re-exporting a type when the '--isolatedModules' flag is provided requires using 'export type'.

  202 | 
  203 | export {
> 204 |   AddPrecompile,
      |   ^
  205 |   CustomPrecompile,
  206 |   DeletePrecompile,
  207 |   getActivePrecompiles,
error Command failed with exit code 1.

I am able to solve this one by changing the erroneous block of code to:

export {
  getActivePrecompiles, precompiles,
  ripemdPrecompileAddress
}
export type {
  AddPrecompile,
  CustomPrecompile,
  DeletePrecompile, PrecompileFunc,
  PrecompileInput
}

Also, I cannot disable --isolatedModules since I am using SWC, which requires it to be enabled, in my project.

Below you can see my tsconfig:

{
  "compilerOptions": {
    "baseUrl": ".",
    "target": "ESNext",
    "lib": [
      "dom",
      "dom.iterable",
      "ESNext"
    ],
    "allowJs": true,
    "skipLibCheck": true,
    "strict": true,
    "forceConsistentCasingInFileNames": true,
    "noEmit": true,
    "esModuleInterop": true,
    "module": "ESNext",
    "moduleResolution": "node",
    "resolveJsonModule": true,
    "isolatedModules": true,
    "jsx": "preserve",
    "incremental": true
  },
  "include": [
    "next-env.d.ts",
    "**/*.ts",
    "**/*.tsx"
  ],
  "exclude": [
    "node_modules"
  ]
}

Any thoughts on these errors? would appreciate your help! :)

thevaizman avatar Sep 19 '22 11:09 thevaizman

In looking at these, the first two look to be easily addressable. Looks like the only difference would be that we would do import * as AsyncEventEmitter from 'async-eventemitter' since we have esModuleInterop:false in our tsconfig. I think that would solve your issues, right?

acolytec3 avatar Sep 22 '22 14:09 acolytec3

@thevaizman Could you try building your app and point your EVM dependency to the head of #2303 and see if that resolves your issue? Updating these import/exports doesn't seem to cause any issues for our internal builds but I don't have an ESM test setup to validate it will work for ESM based projects.

acolytec3 avatar Sep 22 '22 14:09 acolytec3

@thevaizman Could you try building your app and point your EVM dependency to the head of #2303 and see if that resolves your issue? Updating these import/exports doesn't seem to cause any issues for our internal builds but I don't have an ESM test setup to validate it will work for ESM based projects.

Hey @acolytec3, thanks for responding to my issue! :) Unfortunately, after installing from the branch you suggested, I am now getting another error when trying to build:

./node_modules/@ethereumjs/evm/src/evm.ts:153:19
Type error: Property 'events' in type 'EVM' is not assignable to the same property in base type 'EVMInterface'.
  Type 'AsyncEventEmitter<import("/Users/talvaizman/Desktop/Tools/evm.codes/node_modules/@ethereumjs/evm/src/types").EVMEvents>' is not assignable to type 'import("/Users/talvaizman/Desktop/Tools/evm.codes/node_modules/@types/async-eventemitter/index")<import("/Users/talvaizman/Desktop/Tools/evm.codes/node_modules/@ethereumjs/evm/src/types").EVMEvents>'.
    The types returned by 'first(...)' are incompatible between these types.
      Type 'this' is not assignable to type 'AsyncEventEmitter<EVMEvents>'.
        Type 'AsyncEventEmitter<T>' is not assignable to type 'AsyncEventEmitter<EVMEvents>'.
          Types of property 'emit' are incompatible.
            Type '<E extends keyof T>(event: E & string, ...args: Parameters<T[E]>) => boolean' is not assignable to type '<E extends keyof EVMEvents>(event: E & string, ...args: Parameters<EVMEvents[E]>) => boolean'.
              Types of parameters 'args' and 'args' are incompatible.
                Type 'Parameters<EVMEvents[E]>' is not assignable to type 'Parameters<T[E]>'.
                  Type 'EVMEvents[E]' is not assignable to type 'T[E]'.
                    Type 'EVMEvents' is not assignable to type 'T'.
                      'EVMEvents' is assignable to the constraint of type 'T', but 'T' could be instantiated with a different subtype of constraint 'EventMap'.

  151 |   public readonly _transientStorage: TransientStorage
  152 | 
> 153 |   public readonly events: AsyncEventEmitter<EVMEvents>
      |                   ^
  154 | 
  155 |   /**
  156 |    * This opcode data is always set since `getActiveOpcodes()` is called in the constructor
error Command failed with exit code 1.

thevaizman avatar Sep 22 '22 15:09 thevaizman

Apologies, I think I've resolved that build issue now with my latest commit. Can you update to the latest HEAD commit and try it once more?

acolytec3 avatar Sep 22 '22 16:09 acolytec3

Apologies, I think I've resolved that build issue now with my latest commit. Can you update to the latest HEAD commit and try it once more?

Now getting a new error:

./node_modules/@ethereumjs/evm/src/evm.ts:226:23
Type error: This expression is not constructable.
  Type 'typeof AsyncEventEmitter' has no construct signatures.

  224 | 
  225 |   constructor(opts: EVMOpts) {
> 226 |     this.events = new AsyncEventEmitter<EVMEvents>()
      |                       ^
  227 | 
  228 |     this._optsCached = opts
  229 | 
error Command failed with exit code 1.

thevaizman avatar Sep 22 '22 17:09 thevaizman

Hmm, interesting. Can you link me to a repo where I can reproduce this?

acolytec3 avatar Sep 22 '22 19:09 acolytec3

Hmm, interesting. Can you link me to a repo where I can reproduce this?

@acolytec3 sure thing. You can have a look here. This is the branch of my app. You simply need to replace the @ethereumjs/evm dependency with the one you linked me and you will be able to reproduce my error by building with yarn build

thevaizman avatar Sep 22 '22 21:09 thevaizman

@acolytec3 btw, I managed to get more context on the problem by running yarn typecheck:

node_modules/@ethereumjs/evm/src/evm.ts:226:23 - error TS2351: This expression is not constructable.
  Type 'typeof AsyncEventEmitter' has no construct signatures.

226     this.events = new AsyncEventEmitter<EVMEvents>()
                          ~~~~~~~~~~~~~~~~~

  node_modules/@ethereumjs/evm/src/evm.ts:12:1
    12 import * as AsyncEventEmitter from 'async-eventemitter'
       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    Type originates at this import. A namespace-style import cannot be called or constructed, and will cause a failure at runtime. Consider using a default import or import require here instead.

It looks like it still wants the import to be import AsyncEventEmitter from 'async-eventemitter' instead of a namespace import.

thevaizman avatar Sep 23 '22 11:09 thevaizman

I'm honestly not sure how to proceed here. If I do a import the default of AsyncEventEmitter in our code, it is undefined (since the AsyncEventEmitter typing uses the export= syntax instead of export default. From reading this it seems like the only real option for us is the import = require()... or else importing as a namespace. but somehow neither of those is an option when targeting ESM I guess.

acolytec3 avatar Sep 23 '22 15:09 acolytec3

A namespace-style import cannot be called or constructed

@acolytec3 Yeah it seems like the only way to resolve this is actually doing import AsyncEventEmitter from 'async-eventemitter' but this would require you to enable esModuleInterop. Is this out of the question for your project? (genuinely asking as I'm new to all these CommonJS/ES6 shenanigans...)

thevaizman avatar Sep 23 '22 17:09 thevaizman

Unfortunately yes. We just went through the effort of removing that because of concerns we were impacting consumers of our code who might not want to use that setting. That said, we seem to have had the reverse impact on you. 😬 I can explore implementing some alternate typing for this module that resolves the issue, maybe switching it to have a named export. Will do some further research

acolytec3 avatar Sep 23 '22 17:09 acolytec3

Unfortunately yes. We just went through the effort of removing that because of concerns we were impacting consumers of our code who might not want to use that setting. That said, we seem to have had the reverse impact on you. 😬 I can explore implementing some alternate typing for this module that resolves the issue, maybe switching it to have a named export. Will do some further research

That’s completely understandable :) Thanks for taking the time to help with this issue. Hoping to hear good news 🀞

thevaizman avatar Sep 23 '22 18:09 thevaizman

@acolytec3 Just an update, I managed to actually get the thing to build by changing the index.d.ts file so that it contains a named export: export { AsyncEventEmitter } at the end of the file instead of the export = AsyncEventEmitter; they have now. That way, I can import it from your module by changing types.ts and evm.ts imports to: import { AsyncEventEmitter } from 'async-eventemitter'

Not sure if this is something you would do normally, but since async-eventemitter is not maintained anymore (since 2017), maybe you would consider that? Just a thought.

thevaizman avatar Sep 24 '22 15:09 thevaizman

I've actually been trying to do that locally but was continuing to get weird errors. So, will try and put some more thought into on Monday!

acolytec3 avatar Sep 24 '22 15:09 acolytec3

@acolytec3 Sure thing! lmk if I can help somehow and have a great weekend :)

thevaizman avatar Sep 24 '22 15:09 thevaizman

I went back and reread your last comment and I was actually trying something slightly different so I tried your approach this morning but unfortunately I'm having the same sort of errors. The code will all build but our tests fail with a async_eventemitter_1.AsyncEventEmitter is not a constructor error so something is obviously going awry. In your successful build with named exports, does the app actually work or do you get runtime errors trying to instantiate our EVM/VM?

acolytec3 avatar Sep 26 '22 15:09 acolytec3

@acolytec3 that's weird. I am able to both build as well as use all features of the app when applying my changes.

Just to be more clear, these are my changes:

  • node_modules/@types/async-eventemitter/index.d.ts
    • Removed line 20 and append to end of file the following - export {AsyncEventEmitter}
  • node_modules/@ethereumjs/evm/src/types.ts
    • Replace your import type ... with - import {AsyncEventEmitter} from 'async-eventemitter'
  • node_modules/@ethereumjs/evm/src/evm.ts
    • Replace your import * as ... with - import {AsyncEventEmitter} from 'async-eventemitter'

thevaizman avatar Sep 26 '22 15:09 thevaizman

Okay, went back to a clean slate and tried exactly the changes you noted above and I'm still getting the not a constructor error. I'm stumped. :thinking:

acolytec3 avatar Sep 26 '22 18:09 acolytec3

And I confirmed it's working fine when I look at your repo. I cloned it and built locally, using npm link to link it to my local branch with these changes. Seems like ts-node is doing something really screwy when I run our tests.

acolytec3 avatar Sep 26 '22 18:09 acolytec3

Hmm interesting, haven't tried running any of your tests on it, but I wouldn't be surprised if simply trying out different flags when running ts-node is going to solve it. I'll try to take a look at your project's configurations and see if I spot something that might break it.

thevaizman avatar Sep 26 '22 19:09 thevaizman

@acolytec3 Ok after some investigation, I managed to track the problematic sequence that causes ts-node to freak out. The reason for the this.events = new async_eventemitter_1.AsyncEventEmitter(); error we see is because for some reason, when you compile the project, you get the following lines in the /ethereumjs-monorepo/packages/evm/dist/evm.js:45 file:

const async_eventemitter_1 = require("async-eventemitter");

//...

this.events = new async_eventemitter_1.AsyncEventEmitter();

Whereas after using yarn build on my project and looking at the same file in node_modules/@ethereumjs/evm/dist/evm.js I get these lines:

const AsyncEventEmitter = require("async-eventemitter");

// ...

this.events = new AsyncEventEmitter();

I managed to run all the tests just fine after changing these 2 lines of code manually in your project. I have no idea what causes this anomaly between the two projects (maybe some compilation flags?) but these 2 lines of code are definitely the change that breaks the tests in your repo so it's not ts-node's fault I guess.

thevaizman avatar Sep 26 '22 23:09 thevaizman

Interesting... That may be a tricky one. What's especially remarkable is that you're using the same compiled output that I am. We just have some weird double declaration or something somewhere to cause this.

acolytec3 avatar Sep 27 '22 01:09 acolytec3

@acolytec3 Yeah, at first I thought it has to do something with the fact that there are 2 components name AsyncEventEmitter declared in the node_modules/@types/async-eventemitter/index.d.ts file, one is the namespace and the other is the class.

My intuition here says that for some reason, my project seems to "understand" that and it only imports the class from the module, hence the construct-able expression in my evm.js file, whereas your project tries to "find" the class in the namespace and construct it. I thought it might be due to some import * as AsyncEventEmitter expression that is still present somewhere but I couldn't find one...

thevaizman avatar Sep 27 '22 13:09 thevaizman

I definitely see the actual issue now, and I can see typescript intellisense seeing both the namespace and the named export but it's still not clear to me where typescript gets the idea to compile it to async_emitter_1.AsyncEmitter. I remember seeing something like this before somewhere in our repo so will keep digging. This is an odd bug to be sure.

acolytec3 avatar Sep 28 '22 01:09 acolytec3

@thevaizman @acolytec3 have you guys tried differenet typescript versions respectively what are your respective versions? Maybe it is really some compiler bug? πŸ€”

holgerd77 avatar Sep 28 '22 12:09 holgerd77

@holgerd77 @acolytec3 Mine is typescript 4.4.4. I can see that your project has 4.7.4. It doesn't seem to make much difference as I upgraded mine to 4.7.4 using yarn add [email protected] and I got the same compiled node_modules/@ethereumjs/evm/dist/evm.js file as before.

thevaizman avatar Sep 28 '22 14:09 thevaizman

@holgerd77 @acolytec3 Mine is typescript 4.4.4. I can see that your project has 4.7.4. It doesn't seem to make much difference as I upgraded mine to 4.7.4 using yarn add [email protected] and I got the same compiled node_modules/@ethereumjs/evm/dist/evm.js file as before.

πŸ‘ Ah, thanks for investigating!

holgerd77 avatar Sep 30 '22 08:09 holgerd77

@holgerd77 @acolytec3 @jochem-brouwer Hey folks, would love to hear if y'all see any valid solution for this problem on your end. The way I see it, our only option for using the newest version of your library is forking it (with the changes that break the tests) which we would really like to avoid for obvious reasons :/

thevaizman avatar Oct 04 '22 10:10 thevaizman

The AsyncEventEmitter library is MIT licenced, I guess we can just internalize and modify the code base on our side?

WDYGT? πŸ€”

(also has some side-security-benefits)

holgerd77 avatar Oct 04 '22 11:10 holgerd77

(eventually we even add to the Util package, so that we can reuse in both VM and EVM)

holgerd77 avatar Oct 04 '22 11:10 holgerd77