what3words / w3w-node-wrapper

Node wrapper for the What3Words API
https://docs.what3words.com/api/v3/
MIT License
45 stars 10 forks source link

Peer dependencies #45

Closed drjonnicholson closed 2 years ago

drjonnicholson commented 2 years ago

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 2 years ago

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

drjonnicholson commented 2 years ago

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 2 years ago

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,
            }
          }
        },
    },
    };
  6. 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(''); useEffect(() => { if (input.length > 0) { service.autosuggest({ input }) .then(console.log); } }, [input]); return (

setInput(e.currentTarget.value)} />

); }

export default App;

7. Update `package.json`
```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


9. 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.
drjonnicholson commented 2 years ago

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?

drjonnicholson commented 2 years ago

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.

drjonnicholson commented 2 years ago

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.

drjonnicholson commented 2 years ago

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

c5haw commented 2 years ago

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 2 years ago

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

c5haw commented 2 years ago

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