Skip to content
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.

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

Support NPM 8 #1083

Closed
moroine opened this issue Mar 4, 2022 · 9 comments · Fixed by #1084
Closed

Support NPM 8 #1083

moroine opened this issue Mar 4, 2022 · 9 comments · Fixed by #1084
Milestone

Comments

@moroine
Copy link
Contributor

moroine commented Mar 4, 2022

This is a Bug Report

Description

With NPM 8, peer-dependencies are now automatically installed. We decided to move to NPM 8, because we found it easier to manage peer-dependencies.

Unfortunately, serverless-webpack will not find peer-dependencies. I try to take a look at the implementation, and I found some potentials:

  • In packExternalModules we could try to find the installed module version recursively (with NPM 8, peer-dependency cannot conflict by default).
  • Another possibility to explore could be around webpack stat data, to find the module path on disk then look for installed version
  • Add an option to throw when not found instead, forcing the user to install peer-deps

Currently, I'm facing warning, and because it uses "" as the default version when not found it leads to invalid generated package.json.

Here is the generated package.json:

{
  "name": "csv-service",
  "version": "1.0.0",
  "description": "Packaged externals for csv-service",
  "private": true,
  "scripts": {},
  "dependencies": {
    "typed-validators": "^4.4.0",
    "@sentry/serverless": "^6.18.1",
    "abort-controller": "^3.0.0",
    "uuid": "^8.3.2",
    "aws-sdk": "^2.1086.0",
    "csv-parse": "^4.16.3",
    "lodash.isequal": "",
    "redis": "",
    "statsd-client": "",
    "bson": "",
    "@babel/runtime": "",
    "triple-beam": "",
    "winston": "",
    "@sentry/integrations": "",
    "@sentry/node": "",
    "winston-transport": "",
    "mongodb": "^3.7.3",
    "fetch-retry": "",
    "node-fetch": "",
    "react": "",
    "react-router-dom": "",
    "@chakra-ui/react": "",
    "@babel/generator": "",
    "@babel/parser": "",
    "esprima": "",
    "@sentry/react": "",
    "@chakra-ui/icons": "",
    "formik": "",
    "yup": "",
    "escape-string-regexp": "",
    "date-fns": "",
    "lodash.throttle": "",
    "fuse.js": "",
    "react-tiny-virtual-list": ""
  }
}

Additional Data

  • Serverless-Webpack Version you're using: 5.5.4
  • Webpack version you're using: 5.70.0
  • Serverless Framework Version you're using: 2.72.3
  • Operating System: darwin
  • Stack Trace (if available):
Serverless: WARNING: Could not determine version of module lodash.isequal
Serverless: WARNING: Could not determine version of module redis
Serverless: WARNING: Could not determine version of module statsd-client
Serverless: WARNING: Could not determine version of module bson
Serverless: WARNING: Could not determine version of module @babel/runtime
Serverless: WARNING: Could not determine version of module triple-beam
Serverless: WARNING: Could not determine version of module winston
Serverless: WARNING: Could not determine version of module @sentry/integrations
Serverless: WARNING: Could not determine version of module @sentry/node
Serverless: WARNING: Could not determine version of module winston-transport
Serverless: WARNING: Could not determine version of module fetch-retry
Serverless: WARNING: Could not determine version of module node-fetch
Serverless: WARNING: Could not determine version of module react
Serverless: WARNING: Could not determine version of module react-router-dom
Serverless: WARNING: Could not determine version of module react
Serverless: WARNING: Could not determine version of module @chakra-ui/react
Serverless: WARNING: Could not determine version of module @babel/generator
Serverless: WARNING: Could not determine version of module @babel/parser
Serverless: WARNING: Could not determine version of module esprima
Serverless: WARNING: Could not determine version of module @sentry/react
Serverless: WARNING: Could not determine version of module @chakra-ui/icons
Serverless: WARNING: Could not determine version of module formik
Serverless: WARNING: Could not determine version of module yup
Serverless: WARNING: Could not determine version of module escape-string-regexp
Serverless: WARNING: Could not determine version of module date-fns
Serverless: WARNING: Could not determine version of module lodash.throttle
Serverless: WARNING: Could not determine version of module fuse.js
Serverless: WARNING: Could not determine version of module react-tiny-virtual-list
Serverless: Packing external modules: typed-validators@^4.4.0, @sentry/serverless@^6.18.1, abort-controller@^3.0.0, uuid@^8.3.2, aws-sdk@^2.1086.0, csv-parse@^4.16.3, lodash.isequal, redis, statsd-client, bson, @babel/runtime, triple-beam, winston, @sentry/integrations, @sentry/node, winston-transport, mongodb@^3.7.3, fetch-retry, node-fetch, react, react-router-dom, @chakra-ui/react, @babel/generator, @babel/parser, esprima, @sentry/react, @chakra-ui/icons, formik, yup, escape-string-regexp, date-fns, lodash.throttle, fuse.js, react-tiny-virtual-list

 Error ---------------------------------------------------

  Error: npm install failed with code 1
      at ChildProcess.<anonymous> (/Users/moroine/Workspace/Nugit/csv-service/node_modules/serverless-webpack/lib/utils.js:92:16)
      at ChildProcess.emit (node:events:520:28)
      at ChildProcess.emit (node:domain:475:12)
      at maybeClose (node:internal/child_process:1092:16)
      at Process.ChildProcess._handle.onexit (node:internal/child_process:302:5)

     For debugging logs, run again after setting the "SLS_DEBUG=*" environment variable.
@moroine
Copy link
Contributor Author

moroine commented Mar 4, 2022

I managed to find missing deps by running this script:

const { spawn } = require('child_process');
const { readFileSync } = require('fs');
const { satisfies } = require('semver');

function spawnProcess(command, args, options) {
  return new Promise((resolve, reject) => {
    const child = spawn(command, args, options);
    let stdout = '';
    let stderr = '';
    // Configure stream encodings
    child.stdout.setEncoding('utf8');
    child.stderr.setEncoding('utf8');
    // Listen to stream events
    child.stdout.on('data', (data) => {
      stdout += data;
    });
    child.stderr.on('data', (data) => {
      stderr += data;
    });
    child.on('error', (err) => {
      reject(err);
    });
    child.on('close', (exitCode) => {
      if (exitCode !== 0 || stderr.length > 0) {
        reject(new Error(`${command} ${args.join(' ')} failed with code ${exitCode}:\n========== STD OUT ==========\n${stdout}\n========== STD ERR ==========\n${stderr}`, stdout, stderr));
      } else {
        resolve(stdout);
      }
    });
  });
}

async function getDependencyGraph(pkg) {
  const out = await spawnProcess('npm', ['ls', '-a', '--json', '-prod', '--package-lock-only', '-w', pkg]);

  return JSON.parse(out);
}

function isResolved(module) {
  return Boolean(module?.resolved);
}

function findDepModules(graph, pkg, version) {
  const deps = Object.keys(graph.dependencies);

  if (isResolved(graph.dependencies?.[pkg]) && graph.dependencies[pkg].version === version) {
    return graph.dependencies[pkg].dependencies ?? {};
  }

  for (const dep of deps) {
    if (graph.dependencies[dep].dependencies) {
      const found = findDepModules(graph.dependencies[dep], pkg, version);
      if (found) {
        return found;
      }
    }
  }

  return {};
}

function getRequiredDeps(graph, pkg, version) {
  const required = [];
  const depModules = findDepModules(graph, pkg, version);
  const deps = Object.keys(depModules);

  for (const dep of deps) {
    if (dep.startsWith('@nugit/')) {
      required.push(...getRequiredDeps(graph, dep, depModules[dep].version));
    } else {
      required.push(`${dep}@${depModules[dep].version}`);
    }
  }

  return required;
}

async function ensurePeerDepsAreInstalled(packagePath) {
  const missing = new Set();
  const pckJson = JSON.parse(readFileSync(packagePath, 'utf8'));

  const graph = await getDependencyGraph(pckJson.name);
  const deps = getRequiredDeps(graph, pckJson.name, pckJson.version);

  for (const dep of deps) {
    const parts = dep.split('@');
    const pkg = parts.slice(0, -1).join('@');
    const version = parts[parts.length - 1];
    const constraint = pckJson.devDependencies?.[pkg]
      ?? pckJson.dependencies?.[pkg];

    if (constraint === null) {
      missing.add(`${pkg}@^${version}`);
    } else if (!satisfies(version, constraint)) {
      throw new Error(`Peer dependency ${dep} does not match version in package.json ${constraint}`);
    }
  }

  if (missing.size > 0) {
    const command = `npm install --save -w ${pckJson.name} ${Array.from(missing).join(' ')}`;
    throw new Error(`Could not find peer dependency in package.json for ${Array.from(missing).join(' ')}. You can run ${command} to install them.`);
  }
}

ensurePeerDepsAreInstalled('./packages/api/package.json');

@paroxysm
Copy link

FYI, npm8 broke serverless-webpack compilation, at our corporate office, we're being forced to move away from webpack to esbuild because of the lack of urgency on fixing this issue. Moroine has put out a fix that I've tested and works, I would urge the serverless team be more vigilant on this bug.

@j0k3r
Copy link
Member

j0k3r commented Mar 17, 2022

FYI, it seems more easy to revert to NPM7 instead of rewrote your build system using esbuild.

@moroine
Copy link
Contributor Author

moroine commented Mar 17, 2022

@paroxysm it's almost ready. I'm working on unit tests... Starting to go crazy on this, I've migrated to jest

@paroxysm
Copy link

FYI, it seems more easy to revert to NPM7 instead of rewrote your build system using esbuild.

True, however we are using a base set of images that are opinionated on npm8 ,coming from the architects(I would've loved to switch back to npm7). We couldn't control getting this fix merged sooner, but we can control migrating to esbuild to unblock our deployments....
Hopefully this gets fixed soon so other teams don't have to jump thru the hoops I did to unblock.

@moroine
Copy link
Contributor Author

moroine commented Mar 17, 2022

Or you can use @nugit/serverless-webpack as temporary replacement as you stated it's working 🤔

@paroxysm
Copy link

Or you can use @nugit/serverless-webpack as temporary replacement as you stated it's working 🤔

We're looking into publishing that package into our private npm repo for the time being. It would be 1 of two solutions

@moroine
Copy link
Contributor Author

moroine commented Mar 17, 2022

@nugit/serverless-webpack is already published as "@nugit/serverless-webpack": "^5.7.1",

@paroxysm
Copy link

@nugit/serverless-webpack is already published as "@nugit/serverless-webpack": "^5.7.1",

ty! This should help some of the teams that are still behind on migration!

@j0k3r j0k3r added this to the 5.7.0 milestone Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants