@sfcgeorge Wrote a very nice and insightful comment in a PR that was unfortunately abandoned.

Additionally, @Hypnosphi @krainboltgreene and @bcomnes gave feedback on #432.

await prApproval(jamiebuilds/create-react-context#32)

Thanks for doing this @haysclark looks good 👍

➡️ I think "create-react-context": "^0.3.0" would be ideal for now. I don't think dependencies in libraries should ever be locked down to 1 specific version because the chances of version mismatches across libraries in bigger projects are very high and you wouldn't even get security patches. Anyway in our case if they fix it and use semver then in theory they'll release it as a patch, but even if they release as a minor this syntax will cover it too.

There's a chance they'll decide it's been long enough so let's finally bump the package to v1 — then we'd have to add the || "^1.0.0" as you say. I still think it's a good idea to get into the habit of following semver and using the looser version specifier operators though 😄

And for anyone like me who can never remember NPM's package.json syntax for version ranges here are the docs and a handy visual calculator to play with 👾

There's a chance they'll decide it's been long enough so let's finally bump the package to v1 — then we'd have to add the || "^1.0.0"

It's a polyfill, so 1.0.0 won't probably stop supporting older React versions. So it the case you describe, just ^1.0.0 should be enough

I'm happy as long as react(-dom) 17 make it into to the peer deps

I have removed the "WIP" status from this PR, incase a maintainer wants to Approve or Close this PR. If considering merging the commit, please look at my commit note for f23f43d. I have NOT manually tested these changes with React 15, 16, and 17. Possibly an RC package could be released for the community to test against.

The thinking behind the change is driven by the lack of movement from the author/maintainer of create-react-context. I may be mistaken, but from what I gather, create-react-context is a polyfill, ONLY needed by React 15 users. The inclusion of the library in "dependencies" only bloats the bundle size for React 16 and 17 users. Additionally, react and react-dom ARE currently set up to be peerDependencies, which puts the responsibility of maintaining these external libraries on the library users. IMHO, I would argue that any needed polyfills for a specific external library should also fall on the shoulders of that library users; hence commit f23f43d.

It's definitely not a perfect solution, but the scope of this PR quickly balloons into "BREAKING CHANGES" territory when removing the create-react-context library, subsequently ending React 15 support. Naturally, there are a lot of other options, like forking create-react-context, etc; but most alternative solutions are will require a vested project maintainer. The current solution felt like the best middle-ground option... thoughts?

Update:
These two articles were useful if the project maintainer are interested in doing more robust testing with multiple versions of React:

• https://thib.me/testing-with-multiple-versions-of-npm-packages
• https://dev.to/joshx/test-your-npm-package-against-multiple-versions-of-its-peer-dependency-34j4

create-react-context is used directly here: https://github.com/reach/router/blob/master/src/index.js#L26

You can make it optional with something like this, but it will generate a warning in user's webpack builds

let createContext = React.createContext;
if (typeof createContext !== 'function') {
  try {
    createContext = require('create-react-context');
  } catch (e) {
    // log some error
  }
}
const Ctx = createContext(defaultValue);

Please test it using the repo examples if you do it: https://github.com/reach/router/blob/master/CONTRIBUTING.md#developing-the-examples. You may need to remove create-react-context devDependency to test how it works without it

create-react-context appears unmaintained. I recommend its removal, or forking it. I favor removal since its a polyfill for old react versions. Definitely a maintainers call though.

@bcomnes removing create-react-context would mean Reach Router no longer works with React 15. The maintainers would need to decide if they're ok with that, and make a major version release of Reach Router to indicate the breaking change.

I think @haysclark took a great approach, see the updated package.json in this PR. It's now not pinned to just 1 version 👍 and is a peer dependency so it's optional 💯 This means most new users on the latest React can ignore it, they don't need the dependency bloating their codebase and won't have to think about it. For anyone stuck on React 15 they can add create-react-context to get it working again.

Since the peerDependency is optional I'm not sure if there will be a helpful message from NPM. And even if there is it's probably technically still a breaking change because React 15 users will have to add a line to their package.json otherwise their app will break (probably silently). So a major version may still be needed plus a changelog entry telling React 15 users they'll need to add create-react-context.

Happy new year! Any chance we can land this? 🙏 Thank you!

NPM 7 refuses to install this module with React 17 because of the unsatisfiable peer dependency on React 16 from create-react-context, which is unmaintained (last release over 2yr ago). Anyone using NPM 7 and React 17 is going to have a hard time with this module because of this.

I've published a fork of create-react-context with peer dependency on >= 0.14.0 as @hypnosphi/create-react-context. This way, a breaking change can be avoided

Thank you @haysclark I support the adoption of the fork as the upstream repo maintainers have effectively abandoned the project.

I just want to help make this be seen as it's blocking our team from using storybook, which depends on this.

@ryanflorence Any chance that you could look at this PR, or comment?

@ryanflorence Pinging you again on this PR - it's been close to a year since this PR was put up and the community would love to see it approved and released!

@haysclark, can you please yolo this over the fence? I have no control over our jenkins box, and they're using npm 7.