Should index.json be used as well as schedule.json for lts start?

[shadowspawn]

The logic in nv works with the dates from schedule.json to determine the classification of a version. I wonder if index.json should be considered canon for state changes which are considered to required an actual release, in particular for whether a version is considered an LTS version. (The outcome will only be potentially different around the LTS release date.)

From code inspection, I think nv is making the determination based on the lts date from https://raw.githubusercontent.com/nodejs/Release/master/schedule.json at this line:

• nv/index.js

  Line 109 in 8ce8542

  if (now > v.lts) {

However, https://nodejs.org/dist/index.json has its own field for lts which is either false or the release codename. The false makes it seem likely this is not intended to be treated as an lts version even if the date has passed without a new release.

Rather than:

if (now > v.lts)

should the test perhaps be:

if (ver.lts)

(The same argument is likely to apply to the start date for a version, but I have not worked through that logic yet, so raising LTS alone to check my reasoning!)

Edit: the title has changed to reflect a later suggestion to use both sources, rather than one or other as in this original comment.

[wesleytodd]

One of the things nv allows for is to pass a date to evaluate at. This will be important for reproducible builds. If you are using only https://nodejs.org/dist/index.json, being a point in time, it cannot do this. FWIW, I think to get real reproducibility you would need a build receipt with the node version because even the schedule can change, but I wanted nv to at least support a loose definition of reproducibility.

I think we probably want both though. For example, if you do not pass a date, then I think you want the point in time and relying on the schedule lining up and doing date math is less reliable. In that case using just that field does seem like the best solution. Right?

[shadowspawn]

One of the things nv allows for is to pass a date to evaluate at. This will be important for reproducible builds

Interesting. That was not a use case I had considered. Strongly reproducible might be a convenience for build systems, but I am dubious it will work in a reproducible way since both schedule and index may have changed and there are not fine grained timestamps.

For example:

1. nv('current') returns 14.2.0 at 2020-05-06T06:37:44+00:0

2. One minute later a new version of node has been released, and I expect calling again without caching should pick this up:

nv('current') returns 14.2.1 at 2020-05-06T06:38:44+00:0

3. If I call nv('current', myConvertDate('2020-05-06T06:37:44+00:0')) passing in the first time, I don't see a good way of avoiding getting 14.2.1 again since index.json has changed since the first call.

Storing the results of the lookup seems an easy definitive approach if I want to process those results again later?

My personal focus is on the right-now case and expecting current information, so I am not a customer of this feature and may be missing some compelling use cases! What is a use case for a loose definition of reproducibility and a past date?

[shadowspawn]

(Full disclosure: I am very interested in the implementation details because I am looking into how to best resolve the node support aliases in a shell script! I am not a user of nv at this point, and weight my feedback as theoretical rather than as a consumer.)

[shadowspawn]

I have been wondering whether these tests (index vs schedule) make any difference in practice, and think I have a simple example for lts that the schedule dates alone are not a sufficient test for the expected results. See also next comment for a revised suggestion for a possible fix.

There will be a window of time between when the "lts" version is scheduled for release, and when it becomes available. We can conveniently reproduce a check in that time window without needing to mock in test files by looking forward in time instead of back. With code that only checks the schedule, we get an incorrect result with a non-lts version returned.

const nv = require('@pkgjs/nv')

async function lookup(opts) {
  const versions = await nv('lts_latest', opts);
  console.log(versions);
};

const opts = { now: new Date('2020-10-20T00:00:01Z')  };
console.log(opts);
lookup(opts)
  .then(() => console.log('done'))
  .catch(err => console.log(err));

% node mine.js
{ now: 2020-10-20T00:00:01.000Z }
[
  {
    version: '14.2.0',
    major: 14,
    minor: 2,
    patch: 0,
    codename: 'v14',
    versionName: 'v14',
    start: 2020-04-21T00:00:00.000Z,
    lts: 2020-10-20T00:00:00.000Z,
    maintenance: 2021-10-19T00:00:00.000Z,
    end: 2023-04-30T00:00:00.000Z
  }
]
done

[shadowspawn]

To take more of the information into consideration, Instead of only the date check:

if (now > v.lts)

should the test perhaps be:

if (now > v.lts && ver.lts)

This has a nice symmetry with using both sets of information, without introducing two modes of operation.

[wesleytodd]

@shadowspawn sorry for the long wait here. I had an idea while I was working on using this in a new library I was writing at work, and I think it might resolve my reproducibility issue better than only using the now option. #10 (comment)

If we went the route of bundling the built index and schedule into a library it could be pinned along with other deps in the lockifiles. And then because of that, maybe we could add an option to resolve just based on the index.json or like you mention in the last comment if (now > v.lts && ver.lts). Thoughts?

[shadowspawn]

Optionally using a controlled index.json (or equivalent data) would allow the reproducible path to use the same logic as the "live" path, which sounds good to me.

(I don't fully understand the intended controlled approach, but subscribed to #10!)

[wesleytodd]

Ok, I am going to close out this issue in favor of that. I think you, @dominykas, and I agree on the goals of this and I think I can put together a decent PR which get's us there for the 1.0. I will add you as a reviewer when I open that.