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

0.1.9, 0.1.10, 0.2.0 broken due to export from devDependencies #148

Closed
MatthiasKunnen opened this issue Sep 11, 2020 · 3 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@MatthiasKunnen
Copy link

I'm submitting a ...
[x] bug report
[ ] feature request
[ ] question about the decisions made in the repository
[ ] question about how to use this project

Summary
PR #115 broke the module by exporting a member of devDependency graphql-tools.

I recommend against adding graphql-tools to the dependency list for people, like me, who don't need it.

If you switch to ESLint a rule could be added to avoid this in the future.

Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. StackOverflow, personal fork, etc.)

./node_modules/apollo-link-scalars/build/module/index.js
Module not found: Error: Can't resolve 'graphql-tools' in 'node_modules/apollo-link-scalars/build/module'
@eturino eturino pinned this issue Sep 11, 2020
@eturino eturino unpinned this issue Sep 11, 2020
@eturino eturino added the bug Something isn't working label Sep 11, 2020
@eturino eturino self-assigned this Sep 11, 2020
@eturino
Copy link
Owner

eturino commented Sep 11, 2020

You have a point indeed.

1.x exposes this as a dependency instead of a dev-dependency which solves one part of it.

As you said, it is probably better to avoid exposing this altogether. I'll see about create a new major 2.x that removes that dependency, since that would be a breaking change.

For 0.x, since a minor in 0.x world can break, I'll see about release a 0.3.0

Cheers for that

eturino added a commit that referenced this issue Sep 11, 2020
…`makeExecutableSchema`

Including makeExecutableSchema from graphql-tools had an advantage: we could be sure that the
function exported was compatible. There is a downside: many people won't need it and it is big
enough to grant listening to the concerns raised in #148 and remove it altogether

BREAKING CHANGE: remove `makeExecutableSchema` from the exported functions, remove
@graphql-tools/schema from the dependencies

re #148
@MatthiasKunnen
Copy link
Author

For 0.x, since a minor in 0.x world can break, I'll see about release a 0.3.0

Alternatively you could release 0.1.11 and 0.2.1 for people that lock to ~0.1.x/~0.2.x. People do this to prevent breaking changes with minor updates on packages with 0 as major which can happen as you wrote above.

@eturino
Copy link
Owner

eturino commented Sep 12, 2020

yup, that's what I've done.

The breaking change is to stop exporting makeExecutableSchema, so 2.0.0 and 0.3.0 include that

0.2.1 and 1.1.11 move the devDep to dep to fix the bug.

@eturino eturino closed this as completed Sep 12, 2020
eturino added a commit that referenced this issue Sep 15, 2020
eturino added a commit that referenced this issue Sep 15, 2020
eturino added a commit that referenced this issue Sep 15, 2020
…`makeExecutableSchema`

Including makeExecutableSchema from graphql-tools had an advantage: we could be sure that the
function exported was compatible. There is a downside: many people won't need it and it is big
enough to grant listening to the concerns raised in #148 and remove it altogether

BREAKING CHANGE: remove `makeExecutableSchema` from the exported functions, remove
graphql-tools from the dependencies

re #148
eturino added a commit that referenced this issue Sep 15, 2020
eturino added a commit that referenced this issue Sep 15, 2020
…`makeExecutableSchema`

Including makeExecutableSchema from graphql-tools had an advantage: we could be sure that the
function exported was compatible. There is a downside: many people won't need it and it is big
enough to grant listening to the concerns raised in #148 and remove it altogether

BREAKING CHANGE: remove `makeExecutableSchema` from the exported functions, remove
graphql-tools from the dependencies

re #148
eturino added a commit that referenced this issue Sep 15, 2020
…`makeExecutableSchema`

Including makeExecutableSchema from graphql-tools had an advantage: we could be sure that the
function exported was compatible. There is a downside: many people won't need it and it is big
enough to grant listening to the concerns raised in #148 and remove it altogether

BREAKING CHANGE: remove `makeExecutableSchema` from the exported functions, remove
@graphql-tools/schema from the dependencies

re #148
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants