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

'io-ts' and 'io-ts-types' are missing #23

Closed
scink opened this issue Nov 20, 2018 · 8 comments · Fixed by #24
Closed

'io-ts' and 'io-ts-types' are missing #23

scink opened this issue Nov 20, 2018 · 8 comments · Fixed by #24
Assignees
Labels
improvement New feature or request

Comments

@scink
Copy link
Contributor

scink commented Nov 20, 2018

because of remote-data-io.ts import in index.ts packages io-ts and io-ts-types became required. but they are in peerDependencies and NPM doesn't install them. that forces to install unnecessary for project packages manually. removing index.ts will fix it

@scink scink self-assigned this Nov 20, 2018
@sutarmin
Copy link

main in package.json must be returned back to dist/remote-data.js. io-ts type must be imported from remote-data-ts/remote-data-io directly. This will be a breaking change. /cc @mlegenhausen

@raveclassic raveclassic added the bug Something isn't working label Nov 20, 2018
@raveclassic
Copy link
Contributor

This is a regression after #19

@raveclassic
Copy link
Contributor

We need to decide wether we want to keep all modules in the lib consistent.

  • If we remove direct reference from main module (index) to the module importing io-ts then we break consistency, end user will never know where exactly he'll receive "mising module" error.
  • If we remove main module (index) completely (which I'd prefer personally) we'll end up with inability to import directly from package from 'remote-data-ts'. Need to check how will this affect IDE autoimports etc.
  • On the other hand io-ts types is the functionaly of this lib as a whole so I'm not sure wether we should split it into separate incosistent modules. That's why io-ts and io-ts-types were added to peer-dependencies together with added index.ts - this is the main entry point which composes separate modules into the lib. Perhaps we could consider moving the libs from peer-dependencies to dependencies with a relaxed version (like '*').

/cc @mankdev

@raveclassic raveclassic removed the bug Something isn't working label Nov 20, 2018
@raveclassic
Copy link
Contributor

Removing the bug label because it's not as bug, new libs appear in npm warning about missing peer-dependencies.

@raveclassic raveclassic added the improvement New feature or request label Nov 20, 2018
@raveclassic
Copy link
Contributor

I'll also update the commit tree with a BREAKING CHANGE: label and regenerate changelog later.

@mlegenhausen
Copy link
Contributor

@raveclassic I like the third solution.

@raveclassic
Copy link
Contributor

So after a brief discussion it was decided to move peer-dependencies to dependencies.
@scink Could you update #24 (and rename) accordingly?

@scink
Copy link
Contributor Author

scink commented Nov 20, 2018

sure

@scink scink closed this as completed in #24 Nov 20, 2018
raveclassic pushed a commit that referenced this issue Nov 20, 2018
raveclassic pushed a commit that referenced this issue Nov 20, 2018
fixes missing io-ts and io-ts-types

Closes #23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants