what3words/w3w-node-wrapper

Peer dependencies

drjonnicholson opened this issue · 10 comments

Reopening a new issue as @c5haw closed #44 - the issue is not resolved as CrossFetch is not an optional peer.

Example of the error we get on Node v12 is:

image

We are using Axios, and the expectation is that we should be able to build without also adding the cross fetch dependency. Because the peer dependencies does not specify optionality then to get our application to build we have to install both.

This can be solved by adding meta properties to the package json, which is the approach taken in many projects, such as Storybook: https://github.com/storybookjs/storybook/blob/next/addons/docs/package.json

c5haw commented

Please can you provide your package.json and your build script so I can recreate what you are experiencing please.

Sorry, I can't provide that as its a private company repository. But it's essentially a CRACO project.

From the docs: https://docs.npmjs.com/cli/v8/configuring-npm/package-json#peerdependenciesmeta

The reason some of our colleagues are experiencing this is likely that as they are constrained to Node v12 they probably have NPM <= v6, where according to https://docs.npmjs.com/cli/v8/configuring-npm/package-json#peerdependencies from NPM v7 peer dependencies are automatically installed

c5haw commented

I have tried to recreate the issue you're experiencing following the below steps:

  1. nvm install 12
  2. npx create-react-app my-app --template typescript
  3. cd my-app
  4. npm add craco @what3words/api cross-fetch
  5. Add craco.config.js
// craco.config.js
module.exports = {
    reactScriptsVersion: "react-scripts" /* (default value) */,
    webpack: {
        configure: {
          resolve: {
            fallback: {
              // This is only required for webpack 5
              os: false,
            }
          }
        },
    },
};
  1. Update App.tsx
// App.tsx
import React, { useEffect, useState } from 'react';
import what3words from '@what3words/api';
import './App.css';

function App() {
  const service = what3words('YOUR-API-KEY', {}, { transport: 'fetch' })
  const [input, setInput] = useState<string>('');
  useEffect(() => {
    if (input.length > 0) {
      service.autosuggest({ input })
        .then(console.log);
    }
  }, [input]);
  return (
    <div className="App">
      <header className="App-header">
        <input type="text" value={input} onChange={e => setInput(e.currentTarget.value)} />
      </header>
    </div>
  );
}

export default App;
  1. Update package.json
# package.json
{
  "name": "my-app",
  "version": "0.1.0",
  "private": true,
  "dependencies": {
    "@testing-library/jest-dom": "^5.16.2",
    "@testing-library/react": "^12.1.3",
    "@testing-library/user-event": "^13.5.0",
    "@types/jest": "^27.4.1",
    "@types/node": "^16.11.26",
    "@types/react": "^17.0.39",
    "@types/react-dom": "^17.0.11",
    "@what3words/api": "^4.0.5",
    "craco": "^0.0.3",
    "cross-fetch": "^3.1.5",
    "react": "^17.0.2",
    "react-dom": "^17.0.2",
    "react-scripts": "5.0.0",
    "typescript": "^4.6.2",
    "web-vitals": "^2.1.4"
  },
  "scripts": {
    "start": "craco start",
    "build": "craco build",
    "test": "craco test",
    "eject": "react-scripts eject"
  },
  "eslintConfig": {
    "extends": [
      "react-app",
      "react-app/jest"
    ]
  },
  "browserslist": {
    "production": [
      ">0.2%",
      "not dead",
      "not op_mini all"
    ],
    "development": [
      "last 1 chrome version",
      "last 1 firefox version",
      "last 1 safari version"
    ]
  }
}
  1. Ran npm run build (using craco build) - Created a /build directory
npm run build

> my-app@0.1.0 build
> craco build

craco:  *** Cannot find ESLint loader (eslint-loader). ***
Creating an optimized production build...
Compiled with warnings.

src/App.tsx
  Line 13:6:  React Hook useEffect has a missing dependency: 'service'. Either include it or remove the dependency array  react-hooks/exhaustive-deps

Search for the keywords to learn more about each warning.
To ignore, add // eslint-disable-next-line to the line before.

File sizes after gzip:

  57.57 kB  build/static/js/main.d892f085.js
  1.78 kB   build/static/js/787.28cb0dcd.chunk.js
  541 B     build/static/css/main.073c9b0a.css

The project was built assuming it is hosted at /.
You can control this with the homepage field in your package.json.

The build folder is ready to be deployed.
You may serve it with a static server:

  npm install -g serve
  serve -s build

Find out more about deployment here:

  https://cra.link/deployment
  1. Ran npm run start (using craco start) - No errors App ran successfully

If you can provide me with a way to recreate the issue you're experiencing then I would be happy to look into it, but unfortunately with the information you've provided thus far I haven't been able to do so using a CRACO project.


N.B - I also ran the above with axios instead of cross-fetch and this also built successfully. I also ran with nvm use 14 and nvm use 16 to check no other versions of node were causing the issue and all worked without any hitch.

Thanks @c5haw, as pointed at in my last post the issue is related to NPM version rather than Node version.

We are seeing this issue with NPM version 6.13.4.

Which version of npm were you using?

Taken from our build server if we remove cross-fetch as a dependency (axios is still a dependency):
image

We are unable to change the build server configuration at this time.

Also note that I tried removing cross-fetch locally (clearing and reinstalling dependencies). Nothing in my application mentions cross-fetch (we're using axios), but it still gets installed with npm 8.1.0. If I do npm ls cross-fetch I can see it getting automatically installed because of the peer dependency defined in this package:
image

I suspect if you do the same in your example applications above you'll find that both cross-fetch and axios are getting installed despite you adding a dependency for one of them.

Hi @c5haw, any update on your end with this?

c5haw commented

Hi @drjonnicholson - yes we are working on a fix now. We're hoping to get this resolved this week. Apologies for the delay.

c5haw commented

A fix has been published for this now @what3words/api@4.0.6 which should resolve the issue you've been experiencing.

c5haw commented

Closing this issue as it has grown stale with no activity.