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

fix: correct rollup to bundle all but core #846

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

toddbaert
Copy link
Member

@toddbaert toddbaert commented Mar 5, 2024

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

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

@toddbaert toddbaert requested a review from a team as a code owner March 5, 2024 18:30
@toddbaert
Copy link
Member Author

@lukas-reining I think this should work... please verify however you want. Thanks!

Copy link
Member

@lukas-reining lukas-reining left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks good and works with a local angular test app for me.

rollup.config.mjs Show resolved Hide resolved
@toddbaert toddbaert added this pull request to the merge queue Mar 5, 2024
Merged via the queue into main with commit f451e25 Mar 5, 2024
14 checks passed
@toddbaert toddbaert deleted the fix/events-types-bundling branch March 5, 2024 19:07
github-merge-queue bot pushed a commit that referenced this pull request Mar 5, 2024
🤖 I have created a release *beep* *boop*
---


##
[1.13.0](server-sdk-v1.12.0...server-sdk-v1.13.0)
(2024-03-05)


### ✨ New Features

* context propagation
([#837](#837))
([b1abef1](b1abef1))
* maintain state in SDK, add RECONCILING
([#795](#795))
([cfb0a69](cfb0a69))


### 🐛 Bug Fixes

* allow iteration over all event types
([#844](#844))
([411c7b4](411c7b4))
* correct rollup to bundle all but core
([#846](#846))
([f451e25](f451e25))


### 🧹 Chore

* **main:** release core 0.0.27
([#839](#839))
([ccbb1f9](ccbb1f9))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Signed-off-by: OpenFeature Bot <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Mar 5, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.4.14](web-sdk-v0.4.13...web-sdk-v0.4.14)
(2024-03-05)


### ✨ New Features

* maintain state in SDK, add RECONCILING
([#795](#795))
([cfb0a69](cfb0a69))
* suspend on RECONCILING, mem provider fixes
([#796](#796))
([8101ff1](8101ff1))


### 🐛 Bug Fixes

* allow iteration over all event types
([#844](#844))
([411c7b4](411c7b4))
* correct rollup to bundle all but core
([#846](#846))
([f451e25](f451e25))


### 🧹 Chore

* **main:** release core 0.0.27
([#839](#839))
([ccbb1f9](ccbb1f9))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Signed-off-by: OpenFeature Bot <[email protected]>
Signed-off-by: Todd Baert <[email protected]>
Co-authored-by: Todd Baert <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Removing skipLibCheck throws an error
3 participants