Skip to content

chore(JS): Deduplicate FeatureFlags exports#3739

Open
sgaczol wants to merge 1 commit intomainfrom
@sgaczol/flags
Open

chore(JS): Deduplicate FeatureFlags exports#3739
sgaczol wants to merge 1 commit intomainfrom
@sgaczol/flags

Conversation

@sgaczol
Copy link
Copy Markdown
Collaborator

@sgaczol sgaczol commented Mar 9, 2026

Description

This PR deduplicates featureFlags exports.

Closes https://github.com/software-mansion/react-native-screens-labs/issues/699

Changes

  • remove default export from flags.ts
  • update featureFlags imports

Before & after - visual documentation

N/A

Test plan

Checklist

  • Included code example that can be used to test this change.
  • Updated / created local changelog entries in relevant test files.
  • For visual changes, included screenshots / GIFs / recordings documenting the change.
  • For API changes, updated relevant public types.
  • Ensured that CI passes

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@sgaczol sgaczol changed the title refactor(JS): Deduplicate FeatureFlags exports chore(JS): Deduplicate FeatureFlags exports Mar 9, 2026
Comment thread src/flags.ts
},
};

export default featureFlags;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it's breaking change from the perspective of public API, the code looks good, but we shouldn't merge this PR to main as long as we're publishing 4.x from main

Copy link
Copy Markdown
Member

@kkafar kkafar Mar 9, 2026

Choose a reason for hiding this comment

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

Maybe you're right actually. TBH I'm no longer sure myself. We need to define in readme what is the public API - only things exported directly from react-native-screens.

Copy link
Copy Markdown
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

Looks good. I classified this as a breaking change for some reason in the ticket description. Before merging, could you take a look at how Expo & react-navigation uses it. I'm sure that Expo does, react-navigation might not use the feature flags, but not sure.

Please check whether this change will break them or not.

@t0maboro
Copy link
Copy Markdown
Contributor

t0maboro commented Mar 9, 2026

Looks good. I classified this as a breaking change for some reason in the ticket description. Before merging, could you take a look at how Expo & react-navigation uses it. I'm sure that Expo does, react-navigation might not use the feature flags, but not sure.

Please check whether this change will break them or not.

Please keep in mind that feature flags can be used inside end-users' apps, not only downstream libs

Copy link
Copy Markdown
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

Okay, I think @t0maboro is right here. Let's wait with this PR until branch cut, as decided in the ticket description.

@kkafar kkafar added the breaking-change Implementation of this ticket will result in breaking-change to the library label Mar 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change Implementation of this ticket will result in breaking-change to the library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants