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.

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

@lit/context's types conflict with those documented on the context protocol #4609

Closed
bennypowers opened this issue Apr 4, 2024 · 3 comments · Fixed by #4614
Closed

@lit/context's types conflict with those documented on the context protocol #4609

bennypowers opened this issue Apr 4, 2024 · 3 comments · Fixed by #4614

Comments

@bennypowers
Copy link
Collaborator

Which package(s) are affected?

Context (@lit/context)

Description

The context protocol offers some types for context events https://github.com/webcomponents-cg/community-protocols/blob/main/proposals/context.md#definitions

In a typescript project where these types are present, installing and importing @lit/context will break with this error:

node_modules/@lit/context/lib/context-request-event.d.ts:13:9 - error TS2717: Subsequent property declarations must have the same type.  Property ''context-request'' must be of type 'ContextEvent<UnknownContext>', but here has type 'ContextRequestEvent<{ __context__: unknown; }>'.

13         'context-request': ContextRequestEvent<Context<unknown, unknown>>;
           ~~~~~~~~~~~~~~~~~

  lib/context/event.ts:74:5
    74     'context-request': ContextEvent<UnknownContext>;
           ~~~~~~~~~~~~~~~~~
    ''context-request'' was also declared here.


Found 1 error in node_modules/@lit/context/lib/context-request-event.d.ts:13

Reproduction

  1. start a new typescript project, with this tsconfig:
    {
      "compilerOptions": {
        "experimentalDecorators": true,
        "moduleResolution": "NodeNext",
        "module": "NodeNext",
        "target": "ES2022"
      },
    }
  2. copy the type definitions in from the context protocol proposal into ctx.js
  3. install typescript, lit, and lit/context
    {
      "name": "eg",
      "version": "1.0.0",
      "description": "",
      "main": "index.js",
      "type": "module",
      "scripts": {
        "test": "echo \"Error: no test specified\" && exit 1"
      },
      "keywords": [],
      "author": "",
      "license": "ISC",
      "dependencies": {
        "@lit/context": "^1.1.0",
        "lit": "^3.1.2",
        "typescript": "^5.4.3"
      }
    }
  4. copy this into element.js
    import { createContext, provide, consume } from '@lit/context';
    import { LitElement } from 'lit';
    
    import { createContext as createProtocolContext } from './ctx.js'
    
    const context = createContext<number>('number');
    
    const protocolContext = createProtocolContext<number>('protocol-number', 0);
    
    class MyElement extends LitElement {
      @provide({ context }) provided = 0;
      @consume({ context: protocolContext }) consumed = 0;
    }
  5. npx tsc

Observe:

element.ts:12:4 - error TS1271: Decorator function return type is '{ message: "provided type not assignable to consuming field"; provided: unknown; consuming: number; }' but is expected to be 'void' or 'any'.

12   @consume({ context: protocolContext }) consumed = 0;
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

element.ts:12:14 - error TS2741: Property '__context__' is missing in type 'Readonly<Context<number>>' but required in type '{ __context__: unknown; }'.

12   @consume({ context: protocolContext }) consumed = 0;
                ~~~~~~~

  node_modules/@lit/context/development/lib/create-context.d.ts:10:5
    10     __context__: ValueType;
           ~~~~~~~~~~~
    '__context__' is declared here.
  node_modules/@lit/context/development/lib/decorators/consume.d.ts:34:5
    34     context: Context<unknown, ValueType>;
           ~~~~~~~
    The expected type comes from property 'context' which is declared here on type '{ context: { __context__: unknown; }; subscribe?: boolean; }'

node_modules/@lit/context/development/lib/context-request-event.d.ts:13:9 - error TS2717: Subsequent property declarations must have the same type.  Property ''context-request'' must be of type 'ContextEvent<UnknownContext>', but here has type 'ContextRequestEvent<{ __context__: unknown; }>'.

13         'context-request': ContextRequestEvent<Context<unknown, unknown>>;
           ~~~~~~~~~~~~~~~~~

  ctx.ts:69:5
    69     "context-request": ContextEvent<UnknownContext>;
           ~~~~~~~~~~~~~~~~~
    ''context-request'' was also declared here.


Found 3 errors in 2 files.

Errors  Files
     2  element.ts:12
     1  node_modules/@lit/context/development/lib/context-request-event.d.ts:13

Workaround

only use lit's interpretation of context, give up on interop

Is this a regression?

No or unsure. This never worked, or I haven't tried before.

Affected versions

see package.json above

Browser/OS/Node environment

Node: v20.10.0
Typescript and lit: see above package.json

@bennypowers
Copy link
Collaborator Author

bennypowers commented Apr 4, 2024

It looks to me like the runtime types are also incorrect, at least according to the context protocol. So this will not just halt the typescript compiler, but will also fail to interoperate in-browser

Lit's context request event will have a context property of similar type to

interface LitContextRequestEvent extends Event {
  context: { __context__: unknown }
}

but the protocol specifies:

interface ProtocolContextEvent extends Event {
  context: { name: string, initialValue?: unknown },
}

Hoping you can show me how I'm misreading things here

@bennypowers
Copy link
Collaborator Author

Related to #4601

@justinfagnani
Copy link
Collaborator

Thanks for raising this. I'll try your repro locally and see if I find a minimally breaking path forward here.

For initialValue we ended up thinking that wasn't needed and I thought this was changed in https://github.com/webcomponents-cg/community-protocols/pull/36/files#diff-e5fb30dbea21c61c12d5e88dc92a3c10006693d8bc0c7d067b58dd2b8980853fR90 but it looks like I missed the copy in the definitions section. I can put up a PR to fix that if its possible to change your implementation to match.

I think the event type error is cause by the same root problem.

I'm not sure how many other implementations of the protocol there are. We could certainly use some conformance tests or at the very least common types file to catch this kind of thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants