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

Move remx from devDependencies #7174

Merged
merged 1 commit into from
Jul 4, 2021
Merged

Move remx from devDependencies #7174

merged 1 commit into from
Jul 4, 2021

Conversation

yogevbd
Copy link
Collaborator

@yogevbd yogevbd commented Jul 4, 2021

Since we use remx as the store in Navigation mock, it is no longer a dev dependency.

@yogevbd yogevbd merged commit 0398115 into master Jul 4, 2021
@yogevbd yogevbd deleted the yogevbd-patch-1 branch July 4, 2021 09:07
@mikeduminy
Copy link

@yogevbd What is the purpose of the mock?

Can this be a peer dependency instead?

The issue is that this now leaks into all projects that use RNN :(

@mikeduminy
Copy link

Also, if the mock is only used in tests then this definitely qualifies as a devDependency

@yogevbd
Copy link
Collaborator Author

yogevbd commented Aug 25, 2021

@michaelduminy It allows running render tests with RNN. #7140

The mock isn't used only in RNN tests but in our consumers tests as well. Is there an issue with having remx in your dependencies other than that issue?

@lindesvard
Copy link
Contributor

Me and @michaelduminy work at the same company so I can answer this.

We do not explicit have a problem but this adds a lot of extra work for us and bloating our package.json.

Currently we have a big app with a lot of dependencies. We have some checks to see if we get duplicate version of any package, if we do we need to add that version to our resolutions. Just a glimpse how much we need to add.

image

Also we do not want to add extra dependencies to our app even tho this likely will not be added in the production build.

@yogevbd
Copy link
Collaborator Author

yogevbd commented Aug 25, 2021

I Will move it to peerDependencies and will add:

"peerDependenciesMeta": {
    "remx": {
      "optional": true
    }
  }

as it required only when using RNN mocks.

@mikeduminy
Copy link

Thanks so much @yogevbd!

@yogevbd
Copy link
Collaborator Author

yogevbd commented Aug 25, 2021

Happy to help! #7230

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants