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

[Feature] Return "engines" check #1177

Open
2 tasks done
Amareis opened this issue Apr 9, 2020 · 12 comments
Open
2 tasks done

[Feature] Return "engines" check #1177

Amareis opened this issue Apr 9, 2020 · 12 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@Amareis
Copy link

Amareis commented Apr 9, 2020

  • I'd be willing to implement this feature
  • This feature can already be implemented through a plugin

Describe the user story
Simple: I want to pin my node version (and, may be, in future - yarn version) via engines entry in package.json or, may be, in .yarnrc.yml

For example, deps builds are broken at node 12, but on node 10 all is OK. I want to ensure what every who try to start my project cannot use node 12.

Describe the solution you'd like
Just restore old behavior from 1.x, maybe as a (optional) plugin.

Describe the drawbacks of your solution
IDK.

Describe alternatives you've considered
IDK

Additional context
IDK

@Amareis Amareis added the enhancement New feature or request label Apr 9, 2020
@Amareis
Copy link
Author

Amareis commented Apr 9, 2020

@arcanis if you'll provide some basic thoughts, i can try to implement this as plugin.

@DaneTheory
Copy link

Setting engines values for node and yarn within the project root, then further configuring nodeLinker to pnp, pnpMode to loose, and pnpFallBackMode to all currently still enforces engines values to run a project. Are you sure you've set your semver ranges correctly? to force yarn v2, the yarn key value should look something similar to this: >= 2.0.0. Defining node, I haven't had any trouble so far with a simple 12.x or 13.x value that still all works exactly as v1 did. However, this does assume a correctly working initial config of Yarn v2. If you are having problems, I recommend running through what the docs advocate; use of yarn cache clean, yarn rebuild, yarn install --check-cache, etc. Avoid declaring .yarnrc.yml options by hand. I've noticed inconsistent behavior when declaring default values using yarn config set ... and the good ole' fashioned copy and paste. The docs do clearly state that, currently, artifacts are not properly purged in the way we would ultimately want.

@bekliev
Copy link

bekliev commented Mar 11, 2021

Hi there!
Our team migrated from yarn1 to yarn2 and builds after a while started getting broken on specific developers - we found out it the hard way.

So the problem was that yarn2 doesn't respect engines in package.json anymore as yarn1 was doing it.
And some of our devs switched their node version and moved to another project, when they returned to help us - they forgot to change node version back - so their build was always crashing - it took several hours (literally) to find out what is really happened.

If it was intentional please point it out in the docs - because I couldn't find any information about it...
So switched back to yarn1 because of it.

P.S. I subscribed to this issue - and if it resolves - gonna notify our team to migrate again.

@nedkelly
Copy link

We've been having the same issues as @bekliev, we have numerous projects requiring numerous Node/Yarn configurations and moving to Yarn2 we are now seeing our devs spending time trying to figure out what a project won't build. Though we are now aware of the problem it still catches our devs out from time to time, simply enforcing the required engines in package.json would save a lot of time and trouble.

In the meantime I just made a plugin in ./plugin-requirements.check.js:

module.exports = {
  name: `plugin-requirements-check`,
  factory: require => {
    const semver = require('semver');
    const { readFileSync } = require('fs');
    const data = readFileSync('package.json');
    const { engines } = JSON.parse(data.toString());
    const { node } = engines;
    return {
      default: {
        hooks: {
          validateProject(project) {
            if (!semver.satisfies(process.version, node)) {
              throw new Error(
                `The current node version ${process.version} does not satisfy the required version ${node}.`,
              );
            }
          },
        },
      },
    };
  },
};

This produces the following if the node version doesn't satisfy the engines in package.json:

➤ YN0000: ┌ Project validation
➤ YN0001: │ Error: The current node version v12.20.0 does not satisfy the required version >=14.0.0.
    at validateProject (/xxx/xxx/xxx/.devutils/plugin-requirements-check.js:14:21)
    at W.triggerHook (/xxx/xxx/xxx/.yarn/releases/yarn-berry.cjs:2:322167)
    at async /xxx/xxx/xxx/.yarn/releases/yarn-berry.cjs:2:383280
    at async f.startTimerPromise (/xxx/xxx/xxx/.yarn/releases/yarn-berry.cjs:2:396400)
    at async ie.install (/xxx/xxx/xxx/.yarn/releases/yarn-berry.cjs:2:383199)
    at async /xxx/xxx/xxx/.yarn/releases/yarn-berry.cjs:2:109983
    at async Function.start (/xxx/xxx/xxx/.yarn/releases/yarn-berry.cjs:2:395097)
    at async Ue.execute (/xxx/xxx/xxx/.yarn/releases/yarn-berry.cjs:2:109879)
    at async Ue.validateAndExecute (/xxx/xxx/xxx/.yarn/releases/yarn-berry.cjs:2:666150)
    at async Y.run (/xxx/xxx/xxx/.yarn/releases/yarn-berry.cjs:17:3854)
➤ YN0000: └ Completed
➤ YN0000: Failed with errors in 0s 6ms

Hope this helps 👍

@Akryum
Copy link

Akryum commented Sep 2, 2021

I made a repo here:

yarn plugin import https://raw.githubusercontent.com/Akryum/yarn-check-node/main/bundle/check-node-version.js

@devoto13
Copy link
Contributor

devoto13 commented Sep 9, 2021

Ha-ha, funny! I've also made a very similar plugin couple of days back.

https://github.com/devoto13/yarn-plugin-engines

@ericis
Copy link

ericis commented Jul 12, 2022

Ha-ha, funny! I've also made a very similar plugin couple of days back.

https://github.com/devoto13/yarn-plugin-engines

Thanks @devoto13. I've decided to use your plug-in implementation since it has recent updates and seems to have the oldest git commit history.

@rbirkgit

This comment was marked as abuse.

@basickarl

This comment was marked as abuse.

@wiegell
Copy link

wiegell commented Aug 9, 2023

@nedkelly very helpful! I modified it to also run on all scripts and check .nvmrc:

module.exports = {
  name: `plugin-requirements-check`,
  factory: (require) => {
    const semver = require('semver');
    const { readFileSync } = require('fs');
    const data = readFileSync('package.json');
    const { engines } = JSON.parse(data.toString());
    const { node } = engines;
    const nvmrc = readFileSync('.nvmrc').toString().replace('\n', '');
    function checksemver() {
      if (!semver.satisfies(process.version, nvmrc)) {
        throw new Error(
          `The current node version (${process.version}) does not satisfy the required version in .nvmrc (${nvmrc})`
        );
      }
      if (!semver.satisfies(process.version, node)) {
        throw new Error(
          `The current node version (${process.version}) does not satisfy the required version in package.json (${node}.)`
        );
      }
    }
    return {
      default: {
        hooks: {
          validateProject(project) {
            checksemver();
          },
          async wrapScriptExecution(executor) {
            checksemver();
            return executor;
          },
        },
      },
    };
  },
};

@DiFuks
Copy link

DiFuks commented Feb 1, 2024

@wiegell

Your code contains an error that could lead to very serious problems (we've already encountered one).

wrapScriptExecution does not return a Promise<number> as stated in the documentation. You wrap the function in a new promise and return it. As a result, yarn does not await the executor function. Consequently, even if script execution fails (the script returns exit code !== 0), the main yarn process will complete successfully. This could lead to a falsely successful build in the pipeline and to deploying code to production that should not be there. For example, tests that should fail the pipeline in the event of an error will not do so. Try intentionally writing incorrect code (e.g., in the tests) and run yarn test && echo "Test complete"; you will see "Test complete" in the console.

The code needs to be changed as follows:

          validateProject(project) {
            checksemver();
          },
          async wrapScriptExecution(executor) {
            checksemver();
-            return new Promise(executor);
+            return executor;
          },

@wiegell
Copy link

wiegell commented Feb 1, 2024

@DiFuks you're completely right and i have had that problem myself and corrected my code in my project but forgot all about this issue. I have updated my post now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests