0.10.0

[fbartho]

Tracking PR for the 0.10.0 release!

What's Changed

• Breaking: v0.10.0 drops support for ApolloClient v3!
• https://github.com/apollographql/apollo-link-rest/pull/330 by @jackstudd
• Updates to all the dependencies for the modern era

Known Issues

• [ ] There's some sort of packaging issue around ESModules & Source-mapping, possibly related to webpack. -- This is a blocker for shipping.

[fbartho]

@jerelmiller I've implemented the suggested changes that I didn't otherwise ask for help on! Please feel free to give this PR an approval if you think it's ready, or let me know what else you think is a blocker.

It looks like the Apollo permissions on this repo are slightly borked, and despite being the only maintainer, I no longer have the ability to merge a PR without an approving review. (Is that something you can help fix? -- I'm not sure if I can force push to master, but that'll be a blocker soon assuming we get positive feedback on these RCs)

[fbartho]

@jerelmiller oh! and thanks! haha

[jerelmiller]

@fbartho oh weird! Let me ask around internally to see if we can get you maintainer permissions since we (the Apollo Client team) don't maintain this package. They did some internal shuffling some time ago to better onboard engineers to our repos and I bet you got caught up in that. Thanks for flagging!

On the note of the other stuff, I'd be happy to contribute a few more changes!

[jerelmiller]

@fbartho checked with our team and looks like you should have the Maintain role in this repo which should give you the ability to manage pull requests/issues/etc. Can you double check on your end?

[fbartho]

@jerelmiller

I have the ability to merge PRs in general, but I don't have the ability to bypass the branch protection rules that require an approving review.

I also don't have access to the repo settings anymore to adjust the branch protection rules:

[jerelmiller]

Ah I see. I think "Maintainer" gives you some repository settings, but branch protection rules must not be one of them. Let me see if I can get admin access to help out here. Looks like barring that you're at least able to merge PRs. I'll see if I can get that restriction for 1 approving PR removed (or at least one you can bypass).

[jerelmiller]

@fbartho Ok I updated the branch protection rules so that you can bypass requirements to merge. Can you see if this makes it possible to merge now?

[jerelmiller]

FYI I'm still planning to open a PR to address those final few things. I should be able to get to that this afternoon 🙂

[fbartho]

@jerelmiller thanks! I now see the magic checkbox.

[fbartho]

(Reminder to self): Once I get feedback from users, or in a month or so if I don't get feedback, then I'll merge & deploy the full release.

[jerelmiller]

@fbartho by the way, I did get admin rights to this repo so please let me know if you need anything else changed and I'd be happy to help 🙂

[soulfresh]

I'm experiencing what appears to be module resolution issues when using import { RestLink } from 'apollo-link-rest' with .rc.2 in a webpack-based build. I'm not entirely sure whether this is a packaging issue with the library or something specific to our webpack configuration, but wanted to report what I'm seeing since ES module changes were suggested in this thread.

Environment

• Package version: [email protected]
• Bundler: Webpack 5 (via @expo/webpack-config)
• Project: React Native Web application using Expo

Issue 1: Import resolution warning/error

When importing RestLink using the standard import:

import { RestLink } from 'apollo-link-rest'

I get a webpack compilation warning:

  WARNING in ./src/services/report-api/ReportAPI.ts
  export 'RestLink' (imported as 'RestLink') was not found in 'apollo-link-rest'
  (module has no exports)

This shows up as a warning during compilation but causes runtime errors when apollo-link-rest is imported. It's unclear whether this is due to how the ES module re-exports are structured in index.js, or if it's related to webpack's tree-shaking behavior with the "type": "module" package.json setting.

Workaround: Importing directly from the submodule works:

  import { RestLink } from 'apollo-link-rest/restLink'

Issue 2: Source map warnings

When using the direct import workaround above, we get source map warnings:

  WARNING in ../node_modules/apollo-link-rest/restLink.js
  Module Warning (from ../node_modules/source-map-loader/dist/cjs.js):
  Failed to parse source map from '/Users/xxx/node_modules/src/restLink.ts' file: Error: ENOENT: no such file 
  or directory

The source maps appear to reference source files at incorrect paths (missing apollo-link-rest/ in the path). The source TypeScript files are not included in the published npm package, so these source maps can't be resolved.

Workaround: I had to suppress these warnings in our webpack config:

  config.ignoreWarnings = [
    {
      module: /node_modules\/apollo-link-rest/,
      message: /Failed to parse source map/,
    },
  ]

Relevant webpack configuration

Our webpack setup is fairly standard, using Expo's webpack configuration with the following relevant settings:

Module resolution (from Expo):

  resolve: {
    mainFields: ["browser", "module", "main"],
    extensions: [".web.ts", ".web.tsx", ".web.mjs", ".web.js", ".web.jsx",
                 ".ts", ".tsx", ".mjs", ".js", ".jsx", ".json", ".wasm"]
  }

Additional config (our customizations):

  // Add mjs file handling which is required by @graphql-tools
  config.module.rules.push({
    test: /\.mjs$/,
    include: /node_modules/,
    type: 'javascript/auto',
  })

  // Source map warning suppression (workaround)
  config.ignoreWarnings = [
    {
      module: /node_modules\/apollo-link-rest/,
      message: /Failed to parse source map/,
    },
  ]

[fbartho]

@soulfresh Thanks for reporting back! This feels like a blocking issue for stabilizing this release.

I unfortunately don't have time to personally try to solve this problem. I do however have time & I'd be happy to review any PR that fixes this issue targeting this branch, so if you or someone else can help fix the ESModule / build / packaging issues, I'd certainly be happy.

[soulfresh]

I'll give it a try 👍

[jerelmiller]

Got a PR up https://github.com/apollographql/apollo-link-rest/pull/339 that addresses some remaining items and adds an exports field to package.json. @soulfresh I'd be curious if this addresses the issue you're seeing or not.

[soulfresh]

@jerelmiller I gave this a quick try and I wasn't able to get it working with your branch. However, that could be on me. Here's what I've tried:

Yarn install git branch

• yarn add apollo-link-rest@https://github.com/apollographql/apollo-link-rest.git
• update apollo-link-rest version to https://github.com/apollographql/apollo-link-rest.git
• yarn install

Yarn link

• (in apollo-link-rest) yarn build
• (in my project) yarn link ../../apollo-link-rest

With both of the above, I updated my imports from apollo-link-rest/restLink to apollo-link-rest but I get the error "Cannot find module 'apollo-link-rest'" with either approach. Additionally, I can no longer import { RestLink } from 'apollo-link-rest/restLink' which breaks my workaround.

I've tried building the project with yarn install && yarn build:browser but that fails with the following error:

Error: Can't walk dependency graph: Cannot find module '/Users/xxx/apollo-link-rest/lib/bundle.umd.js' from '/Users/xxx/apollo-link-rest/lib/_fake.js'

[fbartho]

@soulfresh thanks for helping test!

As I understand it, this project is definitely configured to require a build step, so I wouldn’t have expected your first attempt (git-based) to have worked unless you added a build-step.

I never intended to support deep-imports to the /restLink / (that’s API surface area that I would consider public-api and need to maintain if we offered it, and I would argue against that)

But your second attempt is something I would have expected to work if I were a consumer of this library.

I have nothing further constructive to add here, but I wanted to demonstrate I am paying attention and interested in further research/fixes.

[soulfresh]

Ah ha! I got this working. And yes, I agree that /restLink should not be considered public.

I can confirm that import { RestLink } from 'apollo-link-rest' is now working for me with this branch after running yarn bundle inside apollo-link-rest and then using yarn link ../apollo-link-rest. I would suggest updating the Contributing section of the README to note that yarn bundle is necessary.

Thanks for this fix! 🙏

[fbartho]

I wouldn’t be opposed to a PR that made the yarn bundle command optional/unnecessary when installing via yarn link or git but I don’t know how hard that is / how much maintenance that would entail.

Glad to hear you got it working for your setup! And good call on the CONTRIBUTING guide tweak. Hopefully I or someone will remember to add to that in the future. :P