Skip to content

Breaking change in latest minor release #39

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.

Sign up for GitHub

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

Jump to bottom
Closed
trevor-scheer opened this issue Jan 25, 2023 · 11 comments
Closed

Breaking change in latest minor release #39

trevor-scheer opened this issue Jan 25, 2023 · 11 comments

Comments

@trevor-scheer
Copy link

trevor-scheer commented Jan 25, 2023 •
edited
Loading

Hey @Ethan-Arrowood, #38 is a breaking change. I'd also argue that just because this lib isn't needed doesn't mean it shouldn't continue to work.

I'm a library author for Apollo Server who supports Node versions 14, 16, 18 (+) and this is part of our runtime code until we drop support for v14. This means that some of our v14 users and all of our 16, 18, + can end up in a broken state.

Can this change be reverted in a patch release please? If not, I'll be pinning to 3.0.x until we drop node v14, but this will continue to effect users of existing apollo server versions if their lockfiles are updated.
@glasser
Copy link

glasser commented Jan 25, 2023 •
edited
Loading

I generally agree with Trevor here.

Moreover, even if you do want to issue a release with a maximum engines constraint (which I think should be a major version change, and doesn't feel necessary), I would think that you'd want the constraint to be <15.4.0 rather than the lower one, as people publishing software for even the latest version of (still in LTS) v14 can't assume that their users are using the experimental flag. "Versions where an API is only available via a top-level experimental flag" is a perfectly reasonable place to use a polyfill.

@trevor-scheer trevor-scheer mentioned this issue Jan 25, 2023
Merged
trevor-scheer added a commit to apollographql/apollo-server that referenced this issue Jan 25, 2023
@trevor-scheer
Pin node-abort-controller (#7326)
e25cb58 
`node-abort-controller` just released `v3.1.0` with a breaking change
that can be pulled in by current Apollo Server users (since the
dependency is careted).

We should pin to the previous latest for now and see what comes of
southpolesteve/node-abort-controller#39
This was referenced Jan 25, 2023
Merged
Closed
trevor-scheer pushed a commit to apollographql/apollo-server that referenced this issue Jan 25, 2023
@github-actions
Version Packages (#7315)
f5607bf 
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @apollo/[email protected]

### Minor Changes

- [#7325](#7325)
[`e0f959a63`](e0f959a)
Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Add
optional `nonFtv1ErrorPaths` to Gateway metrics data. This change is a
prerequisite to:
    -   <apollographql/federation#2242>
    -   <#7136>

## @apollo/[email protected]

### Patch Changes

- [#7316](#7316)
[`37d884650`](37d8846)
Thanks [@renovate](https://github.com/apps/renovate)! - Update
graphql-http dependency

- Updated dependencies
\[[`f246ddb71`](f246ddb),
[`e25cb58ff`](e25cb58)]:
    -   @apollo/[email protected]

## @apollo/[email protected]

### Patch Changes

- [#7314](#7314)
[`f246ddb71`](f246ddb)
Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Add an
`__identity` property to `HeaderMap` class to disallow standard `Map`s
(in TypeScript).

This ensures that typechecking occurs on fields which are declared to
accept a
      `HeaderMap` (notably, the `httpGraphQLRequest.headers` option to
`ApolloServer.executeHTTPGraphQLRequest` and the `http.headers` option
to
`ApolloServer.executeOperation`). This might be a breaking change for
integration authors, but should be easily fixed by switching from `new
    Map<string, string>()` to `new HeaderMap()`.

- [#7326](#7326)
[`e25cb58ff`](e25cb58)
Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Pin
`node-abort-controller` version to avoid breaking change. Apollo Server
users can enter a broken state if they update their package-lock.json
due to a breaking change in a minor release of the mentioned package.

Ref: <southpolesteve/node-abort-controller#39>

- Updated dependencies
\[[`e0f959a63`](e0f959a)]:
    -   @apollo/[email protected]

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@edouardr
Copy link

Agreed with what was said before. For those facing this issue, if you have npm@8 you can use overrides in your package.json and target the previous version.

  "overrides": {
    "node-abort-controller": "3.0.1"
  }

pnpm also supports overrides. For yarn it's resolutions

@BrandonBlanchard
Copy link

BrandonBlanchard commented Jan 26, 2023 •
edited
Loading

It's worth noting that the above override fix will not work if you're using hashicorp/terraform-cdk-action as node-abort-controller is installed from the command line rather than using npm or yarn.

From the Terraform Deploy github action output:

npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '[email protected]',
npm WARN EBADENGINE   required: { node: '<14.7.0' },
npm WARN EBADENGINE   current: { node: 'v16.19.0', npm: '8.19.3' }
npm WARN EBADENGINE }

avandorp added a commit to eclipse-scout/scout.rt that referenced this issue Jan 26, 2023
@avandorp
Build failure due to updated node-abort-controller library
5f839a3 
See southpolesteve/node-abort-controller#39
avandorp added a commit to eclipse-scout/scout.rt that referenced this issue Jan 26, 2023
@avandorp
Build failure due to updated node-abort-controller library
46e641a 
See southpolesteve/node-abort-controller#39
avandorp added a commit to eclipse-scout/scout.rt that referenced this issue Jan 26, 2023
@avandorp
Build failure due to updated node-abort-controller library
f36cb67 
See southpolesteve/node-abort-controller#39

Override only works locally, consumers of package still get wrong dependency: Revert
@tpacent tpacent mentioned this issue Jan 26, 2023
Closed
@shilman shilman mentioned this issue Jan 26, 2023
Closed
@ndelangen
Copy link

This is affecting a lot of storybook users, see:
storybookjs/storybook#20793

@yannbf yannbf mentioned this issue Jan 26, 2023
Merged
@southpolesteve
Copy link
Owner

southpolesteve commented Jan 26, 2023 via email

@ndelangen
Copy link

Thank you so much for the quick reply / assist @southpolesteve ⭐
We appreciate your work!

@Ethan-Arrowood
Copy link
Collaborator

My apologies folks. I didn't think that field would cause a breaking change. Going to revert and release a patch asap. Again, sorry for the headache!

@Ethan-Arrowood
Copy link
Collaborator

v3.1.1 has been published removing the engines property. Please let me know if this fixes everything!

@avandorp
Copy link

eclipse-scout/cli on 23.1 branch builds again, other internal builds are recovering. Thanks for the fast fix!

@yannbf
Copy link
Contributor

yannbf commented Jan 26, 2023

We've tested on our end and Webpack based Storybooks are good now. Thank you so much @Ethan-Arrowood!

@guidev guidev mentioned this issue Jan 26, 2023
Closed
@trevor-scheer
Copy link
Author

Thanks for the quick turnaround @Ethan-Arrowood (and for the PR @yannbf)!

@guilhermep guilhermep mentioned this issue Jan 27, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@glasser @southpolesteve @BrandonBlanchard @yannbf @edouardr @ndelangen @Ethan-Arrowood @avandorp @trevor-scheer