search

PeculiarVentures / pvutils

pvutils is a set of common utility functions used in various Peculiar Ventures Javascript based projects.
Other
7 stars 13 forks source link

New version introduces future syntax in build/utils.js #11

Closed tbranyen closed 2 years ago

tbranyen commented 2 years ago

Looks like this module has build/utils.es.js and build/utils.js, in the new version published 1.1.1 I see in the non-es file:

function getParametersValue(parameters, name, defaultValue) {
    if ((parameters instanceof Object) === false) {
        return defaultValue;
    }
    return parameters[name] ?? defaultValue;
}

Where ?? is not valid ES5-ES2015 syntax. Shouldn't this only occur in the utils.es.js file?

wesleytodd commented 2 years ago

FWIW, even if it is in the ESModule version, adding this syntax is a breaking change because it drops support for a node@12 which is currently LTS.

The most conservative "solution" to this is to re-publish a 1.1.2 which backs out these changes and land this all in 2.0.0. A more "quick" fix would be to just swap the unsupported ?? for a more verbose backward compatible check and keep things in the 1.x range.

wesleytodd commented 2 years ago

I think I am going to retract my statement above, I don't think just changing the syntax is enough. It is HIGHLY unlikely you can do a JS to TS conversion without it being a major semver bump. If I were you I would back it out and re-publish as a2.0.0.

microshine commented 2 years ago

@wesleytodd thank you for that issue I've created a new PR and tested commonjs module on Node v6. It works fine

https://github.com/PeculiarVentures/pvutils/pull/12

microshine commented 2 years ago

@wesleytodd I've published v1.1.2. Please try it

wesleytodd commented 2 years ago

Awesome, thanks for the quick turn around @microshine! We have some teams who it looks like will try out the new version, also I pinned back our originally reported failed build. I was wondering, is there any way to get tags for the previous versions? It looks like you added standard-version now which is awesome, but when I was looking for a quick version to pin back to I was unable to find one because of the missing tags (I ended up going with 1.0.17 which is really old, but is what was in our package.json).

wesleytodd commented 2 years ago

Can confirm that at least for some of our users pulling in 1.1.2 fixed their issues. Thanks again for the quick response @microshine!! Have a great day!