Skip to content
This repository has been archived by the owner on Jan 19, 2019. It is now read-only.

Update requireindex #210

Closed
chrisblossom opened this issue Dec 10, 2018 · 13 comments
Closed

Update requireindex #210

chrisblossom opened this issue Dec 10, 2018 · 13 comments

Comments

@chrisblossom
Copy link

Please change "requireindex": "~1.1.0" to update to minor versions ^1.1.0 and release an updated npm package. I need 1.2.0 in-order to test this plugin in my eslint-config.

Thank you!

@armano2
Copy link
Contributor

armano2 commented Dec 14, 2018

@bradzacher can you close this one: it got updated in #186

@chrisblossom
Copy link
Author

I know it is updated for 1.0/master, but I'm requesting an update before 1.0 is ready...possibly a 0.14.1 release. I probably should have mentioned that in the initial comment.

Also, requireindex is ~ instead of ^ in the PR, which gets the update...but not other non-breaking updates in the future. In my opinion, if you don't trust their semver releases, you should pin the package to a specific version.

@bradzacher
Copy link
Owner

@chrisblossom a healthy distrust of semver is always valid.
A lot of npm devs don't quite understand it and often publish breaking changes under minor bumps.
Some packages even choose to do that (typescript is a good example of that!)

That being said, I do prefer to ^ deps.
And requireindex is rarely ever updated anyways.

I'd like to do a patch release, but it'd be a bit of a PITA with the commit history in the state that it is.
A lesson learned that the 1.0.0 changes should have been on a separate branch 😅

Are you using yarn?
You can handle this pretty easily with yarn's version resolution feature: https://yarnpkg.com/lang/en/docs/selective-version-resolutions/

I think this is the right syntax for it:

{
  "dependencies": {
    "eslint-plugin-typescript": "^0.14.0"
  },
  "resolutions": {
    // forces yarn to ignore any package's requested version, and use v1.2.0
    "requireindex": "1.2.0"
  }
}

@armano2
Copy link
Contributor

armano2 commented Dec 15, 2018

if you are using npm you will have to use this: https://www.npmjs.com/package/npm-force-resolutions

@ficristo
Copy link

@bradzacher I'm sorry to add noise but I saw you fixed this and I'm asking if is it possible to publish a new minor release with the parser upgraded and the new indent rule?
(I'm using it on a toy project of mine so no hurry from my side)

@bradzacher
Copy link
Owner

Unfortunately not.
Master is currently loaded with breaking changes for the 1.0.0 release.

In order to publish a patch release, we'd have to branch off the 0.14.0 tag, bump the dep, test it, and then we can publish it.
We'd also have to rebase master to reorder the commits to include the 0.14.1 release.

It's a lot of work when the 1.0.0 release is very, very close!

@ficristo
Copy link

I was thinking more to release master as is in a 0.15.0 release, since semver doesn't explicitly forbids breaking changes in 0.x.y releases. But I didn't realize 1.0.0 was close. Thank you for your time @bradzacher

@bradzacher bradzacher added this to the 1.0.0 milestone Dec 17, 2018
@bradzacher
Copy link
Owner

since semver doesn't explicitly forbids breaking changes in 0.x.y releases

I would disagree with that statement:
https://semver.org/#spec-item-7

Minor version Y (x.Y.z | x > 0) MUST be incremented if new, backwards compatible functionality is introduced to the public API.

https://semver.org/#spec-item-8

Major version X (X.y.z | X > 0) MUST be incremented if any backwards incompatible changes are introduced to the public API

I will close this as it will be resolved as part of the next release.

@ficristo
Copy link

https://semver.org/#spec-item-4 Major version zero (0.y.z) is for initial development. Anything may change at any time. The public API should not be considered stable.

@bradzacher
Copy link
Owner

Good point, I missed that part of the spec!

Though (in regards to the JS ecosystem) I disagree with that part of the spec because npm/yarn don't differentiate between 0/1 releases, and the default operator (the caret ^) will install it into people's projects, which breaks expectations and causes issues for users.

Esp considering our breaking changes in 1.0.0 will affect everyone, because users must upgrade to our parser for most rules to work.

@ficristo
Copy link

Good thoughts, no problem 😄
I'll patiently wait.

@chrisblossom
Copy link
Author

This is how I've temporarily solved the issue:

npm install --save-dev requireindex@^1 del

// jest typescript testing file

require('../fix-eslint-plugin-typescript');

test('test typescript', () => {})
// fix-eslint-plugin-typescript.js

/**
 * eslint-plugin-typescript is not using a version of 'requireindex'
 * that is compatible with jest.
 *
 * Temporarily fix. Remove when eslint-plugin-typescript updates
 */

'use strict';

const path = require('path');
const fs = require('fs');
const del = require('del');

const eslintPluginTypescriptPath = require.resolve('eslint-plugin-typescript');

const base = path.resolve(
    eslintPluginTypescriptPath,
    '../../node_modules/requireindex'
);

const exists = fs.existsSync(base);

if (exists === true) {
    del.sync(base, { force: true });
}

@j-f1
Copy link
Collaborator

j-f1 commented Dec 17, 2018

According to the repo npm uses to resolve version ranges, versions starting with a 0 are handled properly @bradzacher.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants