kewisch / ical.js

Javascript parser for ics (rfc5545) and vcard (rfc6350) data
https://kewisch.github.io/ical.js/
Mozilla Public License 2.0
996 stars 138 forks source link

Invalid typescript declarations #707

Closed dilyanpalauzov closed 3 months ago

dilyanpalauzov commented 3 months ago

I tried the new type declarations from ical.js, originating from https://github.com/kewisch/ical.js/pull/662:

I do import ICAL from "ical.js". The errors I get are:

In dist/component.d.ts:65: readonly get name(): string; → ../ical.js/dist/types/component.d.ts(65,5): error TS1024: 'readonly' modifier can only appear on a property declaration or index signature.

In event.d.ts:42: constructor(component?: Component | undefined, options: {strictExceptions: boolean; exceptions: Array<Component | Event>; });→ error TS1016: A required parameter cannot follow an optional parameter. Indeed, component is optional, options is not.

period.d.ts:41:static fromJSON(aData: Array<string, string>, aProp: Property, aLenient: boolean): Period; → error TS2314: Generic type 'Array' requires 1 type argument(s).

design.d.ts:10: export function getDesignSet(componentName: string): ICAL.design.designSet; →error TS2503: Cannot find namespace 'ICAL'.

Full list of errors:

../ical.js/dist/types/component.d.ts(65,5): error TS1024: 'readonly' modifier can only appear on a property declaration or index signature. ../ical.js/dist/types/design.d.ts(10,58): error TS2503: Cannot find namespace 'ICAL'. ../ical.js/dist/types/design.d.ts(16,22): error TS2503: Cannot find namespace 'ICAL'. ../ical.js/dist/types/design.d.ts(21,23): error TS2503: Cannot find namespace 'ICAL'. ../ical.js/dist/types/design.d.ts(26,24): error TS2503: Cannot find namespace 'ICAL'. ../ical.js/dist/types/event.d.ts(42,52): error TS1016: A required parameter cannot follow an optional parameter. ../ical.js/dist/types/event.d.ts(174,5): error TS1024: 'readonly' modifier can only appear on a property declaration or index signature. ../ical.js/dist/types/parse.d.ts(26,55): error TS2503: Cannot find namespace 'ICAL'. ../ical.js/dist/types/parse.d.ts(68,59): error TS2304: Cannot find name 'Numeric'. ../ical.js/dist/types/parse.d.ts(93,129): error TS2503: Cannot find namespace 'ICAL'. ../ical.js/dist/types/period.d.ts(41,28): error TS2314: Generic type 'Array' requires 1 type argument(s). ../ical.js/dist/types/property.d.ts(16,48): error TS2503: Cannot find namespace 'ICAL'. ../ical.js/dist/types/property.d.ts(36,5): error TS1024: 'readonly' modifier can only appear on a property declaration or index signature. ../ical.js/dist/types/property.d.ts(42,5): error TS1024: 'readonly' modifier can only appear on a property declaration or index signature. ../ical.js/dist/types/stringify.d.ts(25,53): error TS2503: Cannot find namespace 'ICAL'. ../ical.js/dist/types/stringify.d.ts(38,51): error TS2503: Cannot find namespace 'ICAL'. ../ical.js/dist/types/stringify.d.ts(69,100): error TS2503: Cannot find namespace 'ICAL'. ../ical.js/dist/types/time.d.ts(221,5): error TS1024: 'readonly' modifier can only appear on a property declaration or index signature. ../ical.js/dist/types/timezone_service.d.ts(8,38): error TS1016: A required parameter cannot follow an optional parameter. ../ical.js/dist/types/types.d.d.ts(73,16): error TS2503: Cannot find namespace 'ICAL'.

kewisch commented 3 months ago

@jannikac would you have some time to take a look at these?

jannikac commented 3 months ago

Hi, I created a PR to address most of the issues, however I cant fix errors related to ICAL.design.designSet because as stated by @kewisch that module (at least the jsdoc) is broken and therefore I cant make assumptions on what the correct types should be. The PR is in draft ATM because I'm still waiting on some feedback on two questions regarding the code.

Also while I agree there are errors to be fixed, how are you seeing those errors? I can only see them while viewing the generated d.ts files and not while using the library via an import in a different project like you stated. My setup is vscode, typescript installed with npm and compiling with npx tsc. This returns no errors for me not when compiling or viewing the code in vscode.

dilyanpalauzov commented 3 months ago

Here is how to reproduce:

jannikac commented 3 months ago

Yeah I could reproduce. The issue was that I was testing with npm pack which creates a tarball like what is being distributed via npm. Typescript doesn't check types of imported libraries thats why you wouldn't see the issues when importing the package from npm.

However, I managed to fix all the typescript errors in the previously mentioned PR, even the designSet ones. I tested with my PR and your setup and it worked with no errors!

B2Gdevs commented 3 months ago

How do we create the typings?

I installed the package and just wanted to create the typings to make my linter stop complaining. I ran

tsc --declaration --allowJs --emitDeclarationOnly node_modules/ical.js/dist/ical.js --outDir types --module esnext --target es6

Not sure if that is how I am supposed to do it, I was just thinking that if I could read all the jsdoc in the package I could generate the type and plop it in my app. But I am getting quite a few errors

...
...

node_modules/@types/inquirer/lib/ui/bottom-bar.d.ts:2:27 - error TS2792: Cannot find module '../..'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

2 import inquirer = require("../..");
                            ~~~~~~~

node_modules/@types/inquirer/lib/ui/prompt.d.ts:1:35 - error TS2792: Cannot find module 'rxjs'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

1 import { defer, Observable } from "rxjs";
                                    ~~~~~~

node_modules/@types/inquirer/lib/ui/prompt.d.ts:2:27 - error TS2792: Cannot find module '../..'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

2 import inquirer = require("../..");
                            ~~~~~~~

node_modules/@types/inquirer/lib/utils/events.d.ts:2:28 - error TS2792: Cannot find module 'rxjs'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

2 import { Observable } from "rxjs";
                             ~~~~~~

node_modules/@types/inquirer/lib/utils/utils.d.ts:1:27 - error TS2792: Cannot find module '../..'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

1 import inquirer = require("../..");
                            ~~~~~~~

node_modules/@types/inquirer/lib/utils/utils.d.ts:2:28 - error TS2792: Cannot find module 'rxjs'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

2 import { Observable } from "rxjs";
                             ~~~~~~

node_modules/@types/react/index.d.ts:7:22 - error TS2792: Cannot find module 'csstype'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

7 import * as CSS from "csstype";
                       ~~~~~~~~~

node_modules/@types/react/index.d.ts:4259:14 - error TS2300: Duplicate identifier 'ElementType'.

4259         type ElementType = string | React.JSXElementConstructor<any>;
                  ~~~~~~~~~~~

  node_modules/.pnpm/@types+react@18.2.18/node_modules/@types/react/index.d.ts:3244:14
    3244         type ElementType = string | React.JSXElementConstructor<any>;
                      ~~~~~~~~~~~
    'ElementType' was also declared here.

node_modules/@types/react/index.d.ts:4260:19 - error TS2320: Interface 'Element' cannot simultaneously extend types 'ReactElement<any, any>' and 'ReactElement<any, any>'.
  Named property 'key' of types 'ReactElement<any, any>' and 'ReactElement<any, any>' are not identical.

4260         interface Element extends React.ReactElement<any, any> {}
                       ~~~~~~~

node_modules/@types/react/index.d.ts:4261:19 - error TS2320: Interface 'ElementClass' cannot simultaneously extend types 'Component<any, {}, any>' and 'Component<any, {}, any>'.
  Named property 'componentDidCatch' of types 'Component<any, {}, any>' and 'Component<any, {}, any>' are not identical.

4261         interface ElementClass extends React.Component<any> {
                       ~~~~~~~~~~~~

node_modules/@types/react/index.d.ts:4273:14 - error TS2300: Duplicate identifier 'LibraryManagedAttributes'.

4273         type LibraryManagedAttributes<C, P> = C extends
                  ~~~~~~~~~~~~~~~~~~~~~~~~

  node_modules/.pnpm/@types+react@18.2.18/node_modules/@types/react/index.d.ts:3254:14
    3254         type LibraryManagedAttributes<C, P> = C extends React.MemoExoticComponent<infer T> | React.LazyExoticComponent<infer T>
                      ~~~~~~~~~~~~~~~~~~~~~~~~
    'LibraryManagedAttributes' was also declared here.

node_modules/@types/react/index.d.ts:4280:19 - error TS2320: Interface 'IntrinsicAttributes' cannot simultaneously extend types 'Attributes' and 'Attributes'.
  Named property 'key' of types 'Attributes' and 'Attributes' are not identical.

4280         interface IntrinsicAttributes extends React.Attributes {}
                       ~~~~~~~~~~~~~~~~~~~

node_modules/@types/react/index.d.ts:4281:19 - error TS2320: Interface 'IntrinsicClassAttributes<T>' cannot simultaneously extend types 'ClassAttributes<T>' and 'ClassAttributes<T>'.
  Named property 'key' of types 'ClassAttributes<T>' and 'ClassAttributes<T>' are not identical.

4281         interface IntrinsicClassAttributes<T> extends React.ClassAttributes<T> {}
                       ~~~~~~~~~~~~~~~~~~~~~~~~

Found 232 errors in 28 files.

Errors  Files
     3  node_modules/.pnpm/@types+jest@29.5.11/node_modules/@types/jest/index.d.ts:624
    10  node_modules/.pnpm/@types+node@20.11.5/node_modules/@types/node/globals.d.ts:6
     1  node_modules/.pnpm/@types+react-instantsearch-core@6.26.10/node_modules/@types/react-instantsearch-core/index.d.ts:1
   180  node_modules/.pnpm/@types+react@18.2.18/node_modules/@types/react/index.d.ts:39
     3  node_modules/@types/babel__core/index.d.ts:2
     1  node_modules/@types/babel__generator/index.d.ts:1
     2  node_modules/@types/babel__template/index.d.ts:1
     2  node_modules/@types/babel__traverse/index.d.ts:1
     1  node_modules/@types/inquirer/index.d.ts:17
     1  node_modules/@types/inquirer/lib/objects/choice.d.ts:1
     1  node_modules/@types/inquirer/lib/objects/choices.d.ts:1
     1  node_modules/@types/inquirer/lib/objects/separator.d.ts:1
     2  node_modules/@types/inquirer/lib/prompts/base.d.ts:2
     1  node_modules/@types/inquirer/lib/prompts/checkbox.d.ts:1
     1  node_modules/@types/inquirer/lib/prompts/confirm.d.ts:2
     2  node_modules/@types/inquirer/lib/prompts/editor.d.ts:2
     1  node_modules/@types/inquirer/lib/prompts/expand.d.ts:3
     1  node_modules/@types/inquirer/lib/prompts/input.d.ts:2
     1  node_modules/@types/inquirer/lib/prompts/list.d.ts:2
     1  node_modules/@types/inquirer/lib/prompts/number.d.ts:2
     1  node_modules/@types/inquirer/lib/prompts/password.d.ts:2
     1  node_modules/@types/inquirer/lib/prompts/rawlist.d.ts:1
     1  node_modules/@types/inquirer/lib/ui/baseUI.d.ts:2
     1  node_modules/@types/inquirer/lib/ui/bottom-bar.d.ts:2
     2  node_modules/@types/inquirer/lib/ui/prompt.d.ts:1
     1  node_modules/@types/inquirer/lib/utils/events.d.ts:2
     2  node_modules/@types/inquirer/lib/utils/utils.d.ts:1
     7  node_modules/@types/react/index.d.ts:7
jannikac commented 3 months ago

So typings would be included in the npm package in a future release so you wouldnt have to do anything for that.

Running tsc directly is not supported. Types are only generated by the rollup build process using the typescript plugin. You can generate the types like this:

  1. clone the master branch
  2. run npm i
  3. run npm run build
  4. types are generated in the dist/types directory

Alternatively you can run npm pack after npm run build which creates a tarball with the types included.

dilyanpalauzov commented 3 months ago

The herein mentioned things are now addressed at https://github.com/kewisch/ical.js/pull/712. The comment https://github.com/kewisch/ical.js/pull/712#issuecomment-2179337109 is still relevant.