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

[BUG] Removing skipLibCheck throws an error #845

Closed
gogbog opened this issue Mar 5, 2024 · 3 comments · Fixed by #846 or #847
Closed

[BUG] Removing skipLibCheck throws an error #845

gogbog opened this issue Mar 5, 2024 · 3 comments · Fixed by #846 or #847
Assignees
@lukas-reining
Labels
bug Something isn't working

Comments

@gogbog
Copy link

gogbog commented Mar 5, 2024 •
edited by toddbaert
Loading

Observed behavior

We have a project where we want to use web-sdk and we already have a provider. When we try to use sdk in the project without the --skipLibCheck it doesn't compile.

It throws the following error:

Could not find a declaration file for module 'events'. 'relative-path/angular-first/node_modules/events/events.js' implicitly has an 'any' type.

Try `npm i --save-dev @types/events` if it exists or add a new declaration (.d.ts) file containing `declare module 'events';` [plugin angular-compiler]

By installing npm i --save-dev @types/events, I'm getting the following error:

TS2709: Cannot use namespace 'EventEmitter' as a type. [plugin angular-compiler]
node_modules/@openfeature/web-sdk/dist/types.d.ts:110:37:
protected readonly eventEmitter: EventEmitter;

Expected Behavior

There is no compile error, even tho the skipLibCheck is false.

Steps to reproduce

  1. Create a new angular project - ng new angular-test
  2. Install open-feature SDK - npm i @openfeature/web-sdk
  3. Import Open Feature SDK in some TS file - import { OpenFeature } from '@openfeature/web-sdk'
  4. Switch SkipLibCheck in tsconfig.json to FALSE
@gogbog gogbog added the bug Something isn't working label Mar 5, 2024
@lukas-reining lukas-reining self-assigned this Mar 5, 2024
@lukas-reining
Copy link
Member

lukas-reining commented Mar 5, 2024 •
edited
Loading

Hey @gogbog, thanks for submitting the issue.
I will have a look at this later today.

@toddbaert toddbaert mentioned this issue Mar 5, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Mar 5, 2024
@toddbaert
fix: correct rollup to bundle all but core (#846)
f451e25 
My rollup config was invalid, but clumsily working until a recent change
in how we defined the event emitter type. The impact was the the bundled
types for the web imported `from 'events'` instead of bundling it.

This PR fixes the mis-configuration (see comments).

Here is a screenshot showing the bundled types in the dist now (notice
the import is gone and the emitter is in-lined, no other changes are
present):


![image](https://github.com/open-feature/js-sdk/assets/25272906/8728911d-7c72-4d53-8dc2-d748837a616e)

In server, this type is available from node, so we DON'T need to bundle
it, and in fact, SHOULD import it. This is easily done by importing it
like `from 'node:events'`. I don't think it would be a problem if we
bundled it anyway, but this is more correct.

Fixes: #845

Signed-off-by: Todd Baert <[email protected]>
@toddbaert toddbaert reopened this Mar 5, 2024
@lukas-reining lukas-reining mentioned this issue Mar 5, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Mar 5, 2024
@lukas-reining @toddbaert
feat: use EvenEmitter3 for web-sdk (#847)
861cf83 
<!-- Please use this template for your pull request. -->
<!-- Please use the sections that you need and delete other sections -->

## This PR
<!-- add the description of the PR here -->

Fixes an issue where the `events` node polyfill does not comply to the
`node:events` types.
When trying to use the web OpenFeatureEventEmitter the following error
message comes up, describing that the `events` polyfill's EventEmitter
is incompatible to `node:events` EventEmitter.

```
✘ [ERROR] TS2416: Property 'eventEmitter' in type 'OpenFeatureEventEmitter' is not assignable to the same property in base type 'GenericEventEmitter<ProviderEmittableEvents, Record<string, unknown>>'.
  Type 'EventEmitter' is not assignable to type 'PlatformEventEmitter'.
    Types of property 'addListener' are incompatible.
      Type '(type: string | number, listener: Listener) => EventEmitter' is not assignable to type '(eventName: string | symbol, listener: (...args: any[]) => void) => PlatformEventEmitter'.
        Types of parameters 'type' and 'eventName' are incompatible.
          Type 'string | symbol' is not assignable to type 'string | number'.
            Type 'symbol' is not assignable to type 'string | number'.
```

This PR fixes that issue by not using the `events` anymore and instead
using https://www.npmjs.com/package/eventemitter3

cc @toddbaert 

### Related Issues
<!-- add here the GitHub issue that this PR resolves if applicable -->

Fixes #845

---------

Signed-off-by: Lukas Reining <[email protected]>
Signed-off-by: Todd Baert <[email protected]>
Co-authored-by: Todd Baert <[email protected]>
@toddbaert toddbaert reopened this Mar 5, 2024
@toddbaert
Copy link
Member

toddbaert commented Mar 5, 2024 •
edited
Loading

@gogbog @openfeature/[email protected] is released and @lukas-reining and I believe it should fix your issue, but please confirm and close if it does!

This new version also has the benefit for totaling 220kB instead of 250kB

@gogbog
Copy link
Author

gogbog commented Mar 6, 2024

@toddbaert @lukas-reining Thanks for the fast response. Everything worked as expected. I'm closing the issue.

@gogbog gogbog closed this as completed Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lukas-reining lukas-reining

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
3 participants
@gogbog @toddbaert @lukas-reining