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

Enable strict mode #13

Open
AndreasGassmann opened this issue Sep 27, 2021 · 4 comments
Open

Enable strict mode #13

AndreasGassmann opened this issue Sep 27, 2021 · 4 comments

Comments

@AndreasGassmann
Copy link

I'm trying to integrate this package into AirGap, but I'm running into a lot of build issues after including this package. They seem to be mostly related to strict mode, which we have enabled.

The first error that appears is:

@airgap/angular-core: ✖ Compiling TypeScript sources through NGC
@airgap/angular-core: ERROR: /Users/main/Programming/Papers/airgap-common-components/node_modules/@keystonehq/bc-ur-registry/src/ScriptExpression.ts:1:1: Error encountered in metadata generated for exported symbol 'ScriptExpression': 
@airgap/angular-core:  /Users/main/Programming/Papers/airgap-common-components/node_modules/@keystonehq/bc-ur-registry/src/ScriptExpression.ts:7:27: Metadata collected contains an error that will be reported at runtime: Lambda not supported.
@airgap/angular-core:   {"__symbolic":"error","message":"Lambda not supported","line":6,"character":26}
@airgap/angular-core: An unhandled exception occurred: /Users/main/Programming/Papers/airgap-common-components/node_modules/@keystonehq/bc-ur-registry/src/ScriptExpression.ts:1:1: Error encountered in metadata generated for exported symbol 'ScriptExpression': 
@airgap/angular-core:  /Users/main/Programming/Papers/airgap-common-components/node_modules/@keystonehq/bc-ur-registry/src/ScriptExpression.ts:7:27: Metadata collected contains an error that will be reported at runtime: Lambda not supported.
@airgap/angular-core:   {"__symbolic":"error","message":"Lambda not supported","line":6,"character":26}

Setting "strictMetadataEmit": false seems to fix this, but is not ideal.

I'm not sure what exactly the problem is, but it might be this: https://github.com/KeystoneHQ/ur-registry/blob/main/src/ScriptExpression.ts#L4-L5

Maybe you could instead use a getter for those? See https://stackoverflow.com/questions/57594723/angular-metadata-collected-contains-an-error-that-will-be-reported-at-runtime

After the above error is fixed, I get a lot of errors related to strict mode:

@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/lib/index.ts:8:8 - error TS7016: Could not find a declaration file for module './cbor-sync'. '/Users/main/Programming/Papers/airgap-common-components/node_modules/@keystonehq/bc-ur-registry/src/lib/cbor-sync.js' implicitly has an 'any' type.
@airgap/angular-core: 8 } from './cbor-sync';
@airgap/angular-core:          ~~~~~~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/utils.ts:3:7 - error TS7034: Variable 'alreadyPatchedTag' implicitly has type 'any[]' in some locations where its type cannot be determined.
@airgap/angular-core: 3 const alreadyPatchedTag = [];
@airgap/angular-core:         ~~~~~~~~~~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/utils.ts:6:9 - error TS7005: Variable 'alreadyPatchedTag' implicitly has an 'any[]' type.
@airgap/angular-core: 6     if (alreadyPatchedTag.find((i) => i === tag)) return;
@airgap/angular-core:           ~~~~~~~~~~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/patchCBOR.ts:11:11 - error TS2345: Argument of type '(number | undefined)[]' is not assignable to parameter of type 'number[]'.
@airgap/angular-core:   Type 'number | undefined' is not assignable to type 'number'.
@airgap/angular-core:     Type 'undefined' is not assignable to type 'number'.
@airgap/angular-core: 11 patchTags(registryTags.concat(scriptExpressionTags));
@airgap/angular-core:              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoCoinInfo.ts:39:7 - error TS7053: Element implicitly has an 'any' type because expression of type 'Keys.type' can't be used to index type '{}'.
@airgap/angular-core:   Property '[Keys.type]' does not exist on type '{}'.
@airgap/angular-core: 39       map[Keys.type] = this.type;
@airgap/angular-core:          ~~~~~~~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoCoinInfo.ts:42:7 - error TS7053: Element implicitly has an 'any' type because expression of type 'Keys.network' can't be used to index type '{}'.
@airgap/angular-core:   Property '[Keys.network]' does not exist on type '{}'.
@airgap/angular-core: 42       map[Keys.network] = this.network;
@airgap/angular-core:          ~~~~~~~~~~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoKeypath.ts:44:11 - error TS7034: Variable 'components' implicitly has type 'any[]' in some locations where its type cannot be determined.
@airgap/angular-core: 44     const components = [];
@airgap/angular-core:              ~~~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoKeypath.ts:54:28 - error TS7005: Variable 'components' implicitly has an 'any[]' type.
@airgap/angular-core: 54     map[Keys.components] = components;
@airgap/angular-core:                               ~~~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoKeypath.ts:88:46 - error TS2454: Variable 'sourceFingerprint' is used before being assigned.
@airgap/angular-core: 88     return new CryptoKeypath(pathComponents, sourceFingerprint, depth);
@airgap/angular-core:                                                 ~~~~~~~~~~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:40:11 - error TS2564: Property 'master' has no initializer and is not definitely assigned in the constructor.
@airgap/angular-core: 40   private master: boolean;
@airgap/angular-core:              ~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:41:11 - error TS2564: Property 'privateKey' has no initializer and is not definitely assigned in the constructor.
@airgap/angular-core: 41   private privateKey: boolean;
@airgap/angular-core:              ~~~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:42:11 - error TS2564: Property 'key' has no initializer and is not definitely assigned in the constructor.
@airgap/angular-core: 42   private key: Buffer;
@airgap/angular-core:              ~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:43:11 - error TS2564: Property 'chainCode' has no initializer and is not definitely assigned in the constructor.
@airgap/angular-core: 43   private chainCode: Buffer;
@airgap/angular-core:              ~~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:44:11 - error TS2564: Property 'useInfo' has no initializer and is not definitely assigned in the constructor.
@airgap/angular-core: 44   private useInfo: CryptoCoinInfo;
@airgap/angular-core:              ~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:45:11 - error TS2564: Property 'origin' has no initializer and is not definitely assigned in the constructor.
@airgap/angular-core: 45   private origin: CryptoKeypath;
@airgap/angular-core:              ~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:46:11 - error TS2564: Property 'children' has no initializer and is not definitely assigned in the constructor.
@airgap/angular-core: 46   private children: CryptoKeypath;
@airgap/angular-core:              ~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:47:11 - error TS2564: Property 'parentFingerprint' has no initializer and is not definitely assigned in the constructor.
@airgap/angular-core: 47   private parentFingerprint: Buffer;
@airgap/angular-core:              ~~~~~~~~~~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:48:11 - error TS2564: Property 'name' has no initializer and is not definitely assigned in the constructor.
@airgap/angular-core: 48   private name: string;
@airgap/angular-core:              ~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:49:11 - error TS2564: Property 'note' has no initializer and is not definitely assigned in the constructor.
@airgap/angular-core: 49   private note: string;
@airgap/angular-core:              ~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:72:7 - error TS2322: Type 'number | undefined' is not assignable to type 'number'.
@airgap/angular-core:   Type 'undefined' is not assignable to type 'number'.
@airgap/angular-core: 72       depth = this.getOrigin().getComponents().length || this.getOrigin().getDepth();
@airgap/angular-core:          ~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:88:31 - error TS2454: Variable 'index' is used before being assigned.
@airgap/angular-core: 88     indexBuffer.writeUInt32BE(index, 0);
@airgap/angular-core:                                  ~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:115:5 - error TS2322: Type 'boolean | undefined' is not assignable to type 'boolean'.
@airgap/angular-core:   Type 'undefined' is not assignable to type 'boolean'.
@airgap/angular-core: 115     this.privateKey = args.isPrivateKey;
@airgap/angular-core:         ~~~~~~~~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:117:5 - error TS2322: Type 'Buffer | undefined' is not assignable to type 'Buffer'.
@airgap/angular-core:   Type 'undefined' is not assignable to type 'Buffer'.
@airgap/angular-core: 117     this.chainCode = args.chainCode;
@airgap/angular-core:         ~~~~~~~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:118:5 - error TS2322: Type 'CryptoCoinInfo | undefined' is not assignable to type 'CryptoCoinInfo'.
@airgap/angular-core:   Type 'undefined' is not assignable to type 'CryptoCoinInfo'.
@airgap/angular-core: 118     this.useInfo = args.useInfo;
@airgap/angular-core:         ~~~~~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:119:5 - error TS2322: Type 'CryptoKeypath | undefined' is not assignable to type 'CryptoKeypath'.
@airgap/angular-core:   Type 'undefined' is not assignable to type 'CryptoKeypath'.
@airgap/angular-core: 119     this.origin = args.origin;
@airgap/angular-core:         ~~~~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:120:5 - error TS2322: Type 'CryptoKeypath | undefined' is not assignable to type 'CryptoKeypath'.
@airgap/angular-core:   Type 'undefined' is not assignable to type 'CryptoKeypath'.
@airgap/angular-core: 120     this.children = args.children;
@airgap/angular-core:         ~~~~~~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:121:5 - error TS2322: Type 'Buffer | undefined' is not assignable to type 'Buffer'.
@airgap/angular-core:   Type 'undefined' is not assignable to type 'Buffer'.
@airgap/angular-core: 121     this.parentFingerprint = args.parentFingerprint;
@airgap/angular-core:         ~~~~~~~~~~~~~~~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:122:5 - error TS2322: Type 'string | undefined' is not assignable to type 'string'.
@airgap/angular-core:   Type 'undefined' is not assignable to type 'string'.
@airgap/angular-core: 122     this.name = args.name;
@airgap/angular-core:         ~~~~~~~~~
@airgap/angular-core: ../../node_modules/@keystonehq/bc-ur-registry/src/CryptoHDKey.ts:123:5 - error TS2322: Type 'string | undefined' is not assignable to type 'string'.
@airgap/angular-core:   Type 'undefined' is not assignable to type 'string'.

To get rid of those errors, we would have to disable strict mode in our project.

I think you will be able to reproduce most of those errors if you enable strict mode in your tsconfig.json. We have the following settings:

    "strict": true /* Enable all strict type-checking options. */,
    "strictNullChecks": true /* Enable strict null checks. */,
    "strictFunctionTypes": true /* Enable strict checking of function types. */,
    "strictBindCallApply": true /* Enable strict 'bind', 'call', and 'apply' methods on functions. */,
    "strictPropertyInitialization": true /* Enable strict checking of property initialization in classes. */,
    "alwaysStrict": true /* Parse in strict mode and emit "use strict" for each source file. */,
    /* Additional Checks */
    "noImplicitAny": true /* Raise error on expressions and declarations with an implied 'any' type. */,
    "noImplicitThis": true /* Raise error on 'this' expressions with an implied 'any' type. */,
    "noImplicitReturns": true /* Report error when not all code paths in function return a value. */,
    "noUnusedLocals": true /* Report errors on unused locals. */,
    "noUnusedParameters": true /* Report errors on unused parameters. */,
    "noFallthroughCasesInSwitch": true /* Report errors for fallthrough cases in switch statement. */,
    "forceConsistentCasingInFileNames": true
  "angularCompilerOptions": {
    "enableIvy": false,
    "strictInjectionParameters": true,
    "fullTemplateTypeCheck": true,
    "strictTemplates": true,
    "strictInputAccessModifiers": true,
    "strictMetadataEmit": true
  },
@aaronisme
Copy link
Contributor

Thanks for raise this, we will look into this. Currently having a big backlog for development, need find some time to address this. A PR is also really welcome :)

mcmire added a commit to MetaMask/metamask-extension that referenced this issue Feb 2, 2022
@mcmire
WIP - TypeScript support
a1c8ea1 
I've got this working so that:

* TypeScript files can be added to folder, and they will be compiled as
  part of the build system
* TypeScript files can import JavaScript files
* JavaScript files can import TypeScript files
* Missing types can be added in `dependencies.d.ts`

Here's what doesn't work yet:

- [ ] The @keystonehq/ur-registry has type issues, which is causing the
  build system to fail. It seems that the authors have not decided to
  turn on strict mode, whereas we are using strict mode. There is an
  open issue around that here:
  <KeystoneHQ/ur-registry#13>. We probably
  need to lead the charge on this and help them out. We have connections
  to the Keystone devs so we could probably ping them directly when we
  have the work done.
This was referenced Feb 2, 2022
Merged
Open
@mcmire
Copy link

mcmire commented Feb 16, 2022 •
edited
Loading

We are in the process of converting MetaMask into TypeScript and we are getting this error too. Upon further investigation, it seems like this package contains both .ts and .js files. I don't think this is the recommended way to publish a TypeScript-capable package to NPM. I am trying to find an official guide on this, but I believe the standard way of doing this is to omit the full .ts files in favor of just declaration (.d.ts) files. That would obviate the need to turn off strict mode. Is this something that y'all are working on? If not would you be opposed to us working on this for you?

@mcmire mcmire mentioned this issue Feb 16, 2022
Merged
@AndreasGassmann
Copy link
Author

@mcmire We (the AirGap team) are currently not working on this. We disabled strict mode temporarily in one of our modules to make it work. We have it in our backlog to create a PR here, but I don't think we will get to it in the next few weeks.

I'm sure the Keystone team would be happy if you could take a look and create a PR.

@mcmire
Copy link

mcmire commented Oct 10, 2022

Do we think this is fixed now? I noticed this commit come through a while back: 63f6896#diff-b55cdbef4907b7045f32cc5360d48d262cca5f94062e353089f189f4460039e0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mcmire @AndreasGassmann @aaronisme