Skip to content

update runtime dependencies, peer dependencies#115

Merged
EmilianoSanchez merged 2 commits intosplitio:developmentfrom
lfender6445:update_deps
Dec 16, 2022
Merged

update runtime dependencies, peer dependencies#115
EmilianoSanchez merged 2 commits intosplitio:developmentfrom
lfender6445:update_deps

Conversation

@lfender6445
Copy link
Copy Markdown
Contributor

@lfender6445 lfender6445 commented Nov 11, 2022

React SDK

What did you accomplish?

  • according to
    import { SplitFactory as SplitSdk } from '@splitsoftware/splitio';
    '@splitsoftware/splitio' is a runtime dependency
  • better support build systems that run npm prune --production

How do we test the changes introduced in this PR?

Unfortunately I had trouble getting the test suite to pass. I fixed one broken path, but there are 3 more failures, all unrelated I think. Given there have been no logical changes, happy path testing should be fine.

Extra Notes

  • Added react-dom to peer deps
  • Some types were also incorrectly labeled as runtime dependencies, they have been moved as well
  • It appears I am generating a different lockfile format than the author. If there are npm version constraints they should be expressed in the package.json but I did not see any

- according to https://github.com/splitio/react-client/blob/1a58f06b91c63cac2f4032fa3c338573fb30a615/src/utils.ts#L2
'@splitsoftware/splitio' is a runtime dependency
- react peer dependency rules should have a sibling rule for react-dom as well
@agustinona
Copy link
Copy Markdown

Hi @lfender6445 , thanks for the contribution!

Our engineering team will review this and take it into consideration for a future release.

@EmilianoSanchez EmilianoSanchez changed the base branch from master to development December 16, 2022 17:39
@EmilianoSanchez EmilianoSanchez merged commit acd9988 into splitio:development Dec 16, 2022
@EmilianoSanchez
Copy link
Copy Markdown
Contributor

Hi @lfender6445 ,

We have merged your changes, but will do some refactors before releasing a new version of the SDK.

Thank you

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.

3 participants