uuosio / ascdk

MIT License
12 stars 9 forks source link

Failing ExtendedSymbol not found #59

Closed jafri closed 2 years ago

jafri commented 2 years ago

I have published the failing branch 83ef1f249d2117b5faec82282f77650d3d189f20

Running command yarn build:allow2 in examples

It is currently failing with:

Build Starting ······
++++++writeFile: ../../allow2/target/allow2.contract.abi
Build progressing. Generating target files ······
ERROR TS2304: Cannot find name 'ExtendedSymbol'.

     token!: ExtendedSymbol;
             ~~~~~~~~~~~~~~
 in allow2/allow2.contract.ts(51,13)

ERROR TS2304: Cannot find name 'ExtendedSymbol'.

         token: ExtendedSymbol | null = null,
                ~~~~~~~~~~~~~~
 in allow2/allow2.contract.ts(56,16)

ERROR TS2304: Cannot find name 'ExtendedSymbol'.

             let obj = new ExtendedSymbol();
                           ~~~~~~~~~~~~~~
 in allow2/allow2.contract.ts(77,27)

ERROR TS2322: Type 'i32' is not assignable to type '~lib/as-chain/serializer/Packer'.

             dec.unpack(obj);
                        ~~~
 in allow2/allow2.contract.ts(78,24)

ERROR TS2339: Property 'token' does not exist on type 'allow2/allow2.contract/settokenAction'.

             this.token = obj;
                  ~~~~~
 in allow2/allow2.contract.ts(79,18)

ERROR TS2339: Property 'token' does not exist on type 'allow2/allow2.contract/settokenAction'.

             mycontract.settoken(args.token,args.isAllowed,args.isBlocked)
                                      ~~~~~
 in allow2/allow2.contract.ts(157,38)

ERROR TS2339: Property 'token' does not exist on type 'allow2/allow2.contract/settokenAction'.

         size += this.token.getSize();
                      ~~~~~
 in allow2/allow2.contract.ts(88,22)

ERROR TS2339: Property 'token' does not exist on type 'allow2/allow2.contract/settokenAction'.

         enc.pack(this.token);
                       ~~~~~
 in allow2/allow2.contract.ts(67,23)

FAILURE 8 compile error(s)
Build Done. Targets generated. Target directory: allow2/target.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
learnforpractice commented 2 years ago

import { ExtendedSymbol } from 'as-chain' Fix this issue.

See the generated file in target/allow2.contract.ts, it's missing ExtendedSymbol

I don't have a better solution for this kind of issue currently.

I think it will be better to avoid extending a Contract class with action in it.

jafri commented 2 years ago

@learnforpractice how about a list that we replace like

Replace Name with _chain.Name Replace ExtendedSymbol with _chain.ExtendedSymbol

Another approach is to simply add the imports automatically (doesn't increase WASM size)

The following code in optimizeCode fixes it (not ideal fix though, best fix would be to generate actionTpl in the imported file, and then import the action needed from there):

    // Add missing functions
    const fullList = ['Name', 'Table', 'InlineAction', 'Singleton', 'ExtendedSymbol', 'ExtendedAsset', 'Asset', 'Symbol', 'U128', 'IDXDB', 'IDX128']
    const imports = code.match(/^(\s+)?import.*?.('|").*?('|")/gms)
    const extractImports = (imp) => imp.split('{')[1].split('}')[0].split(',').map(_ => _.trim())
    const allImports = imports.reduce((acc, imp) => ~imp.indexOf('{') ? acc.concat(extractImports(imp)) : acc, [])
    const missingImports = fullList.filter(_ => !allImports.includes(_))
    if (missingImports.length) {
        code = `import { ${missingImports.join(', ')} } from "as-chain"\n` + code
    }
jafri commented 2 years ago

One of the core features of this library is how extensible it is, recommending to not extend a contract class kills one of its main features. Ability to extend contracts without having to import ghost elements like ExtendedSymbol is the parent contract is definitely needed

learnforpractice commented 2 years ago

That's an ugly fix. Must find out a better solution. The key point is to find out all the missing symbols used by the generated code.

learnforpractice commented 2 years ago
Replace Name with _chain.Name
Replace ExtendedSymbol with _chain.ExtendedSymbol

That sounds like a better solution.

learnforpractice commented 2 years ago

Fixed by b6e5cbc3f185aa1084492018ef3bed1f9d8b91a8

jafri commented 2 years ago

Fixed, beauty

@learnforpractice do you think we could add a way to tell transformer to compile an imported library too?

For example if I publish the allow contract as an npm package, when I run ascdk on a contract importing allow like:

import { AllowContract } from 'allow'

the transformer won't pick up that AllowContract needs to be compiled too, any way to resolve without having to do like:

import { AllowContract } from './node_modules/allow'
learnforpractice commented 2 years ago

Yeah, that's a new problem. Need to dig into the transform source code again.

jafri commented 2 years ago

@learnforpractice hope this helps

allow library in package.json states

"main": "./assembly/index.ts"

So readFile ascdk is looking for :

'../node_modules/allow/assembly/index.ts'

However in sourceModifier.fileExtMap it has '~lib/allow/index.ts'

Here was a ugly hacky fix I tried adding here that worked:

        if (relativePath.indexOf('allow') !== -1) {
            if (!sourceModifier.fileExtMap.has(relativePath)) {
                const alternativePath = relativePath.replace(/.*node_modules/, '~lib').replace('/assembly', '')
                if (sourceModifier.fileExtMap.has(alternativePath)) {
                    relativePath = alternativePath
                }
            }
        }

        if (sourceModifier.fileExtMap.has(relativePath)) {
learnforpractice commented 2 years ago

Yeah, sounds like you know how to fix it.

jafri commented 2 years ago

I actually am not sure of the best way to fix it, is this hacky fix the right way?

learnforpractice commented 2 years ago

That's the right place to fix it but needs to be improved to make it handle common cases I think.

jafri commented 2 years ago

This works well

        // Transform library imports
        if (relativePath.indexOf('/as-chain/') === -1 && !sourceModifier.fileExtMap.has(relativePath)) {
            const alternativePath = relativePath.replace(/.*node_modules/, '~lib').replace('/assembly', '')
            if (sourceModifier.fileExtMap.has(alternativePath)) {
                relativePath = alternativePath
            }
        }
jafri commented 1 year ago

Yes but that’s not clear to a user at all, why would they import ExtendedSymbol when it’s not used in the current file..


From: learnforpractice @.> Sent: Wednesday, April 13, 2022 10:08:43 PM To: uuosio/ascdk @.> Cc: Syed Jafri @.>; Author @.> Subject: Re: [uuosio/ascdk] Failing ExtendedSymbol not found (Issue #59)

[△EXTERNAL]

import { ExtendedSymbol } from 'as-chain' Fix this issue.

See the generated file in target/allow2.contract.ts, it's missing ExtendedSymbol

I don't have a better solution for this kind of issue currently.

I think it will be better to avoid extending a Contract class with action in it.

— Reply to this email directly, view it on GitHubhttps://github.com/uuosio/ascdk/issues/59#issuecomment-1098689185, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADILGLJDY2DFOWIGOGZTMTDVE6K4XANCNFSM5TMA25XA. You are receiving this because you authored the thread.Message ID: @.***>

learnforpractice commented 1 year ago

Excuse me, what are you talking about? This issue has been fix. Is there a new problems?

jafri commented 1 year ago

I think there is some issue with github @learnforpractice its re-sending old email replies

learnforpractice commented 1 year ago

Ok, got it.