Skip to content

Commit

Permalink
fix: correct rollup to bundle all but core (#846)
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
toddbaert authored Mar 5, 2024
1 parent b1abef1 commit f451e25
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 11 deletions.
2 changes: 1 addition & 1 deletion packages/client/src/events/open-feature-event-emitter.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { GenericEventEmitter } from '@openfeature/core';
import EventEmitter from 'events';
import { EventEmitter } from 'events';
import { ProviderEmittableEvents } from './events';
/**
* The OpenFeatureEventEmitter can be used by provider developers to emit
Expand Down
2 changes: 1 addition & 1 deletion packages/server/src/events/open-feature-event-emitter.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { GenericEventEmitter } from '@openfeature/core';
import EventEmitter from 'events';
import { EventEmitter } from 'node:events';
import { ProviderEvents } from './events';

/**
Expand Down
16 changes: 7 additions & 9 deletions rollup.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,15 @@ export default {
input: "./src/index.ts",
output: {
file: './dist/types.d.ts',
format: 'es', // module format doesn't really matter here since output i
format: 'es', // module format doesn't really matter here since output is types
},
// function indicating which deps should be considered external: external deps will NOT have their types bundled
external: (id) => {
// bundle everything but '@openfeature/core', which is a peer
return id === '@openfeature/core';
},
external: [
// function indicating which deps should be considered external: non-external deps will have their types bundled
(id) => {
// bundle 'events' types
return id !== 'events';
}
],
plugins: [
// use the rollup override tsconfig (applies equivalent in each sub-packages as well)
dts({tsconfig: './tsconfig.rollup.json'}),
dts({tsconfig: './tsconfig.rollup.json', respectExternal: true }),
],
};

0 comments on commit f451e25

Please sign in to comment.