Various maintenance - 0.3.0-dev

[kethinov]

• More package.json refactoring.
• Removed .npmignore in favor of files entry in package.json.
• Renamed dom-parser.js to domParser.js.
• Replaced CHANGELOG with more rigorous file.
• Replaced LICENSE with more rigorous file.
• Removed component.json (deprecated package manager https://github.com/componentjs/guide)
• proof devDep updated to latest.

Closes #2.
Closes #12.
Closes #20.

Requesting review from @brodybits @timbru31.

Pending approval and merge, we should release this as 0.2.2.

[brodybits]

Thanks @kethinov for your work on supporting this package, I am sorry that I could not provide very much active support for the recent issues and contributions.

I would personally favor a couple things pretty strongly:

• new 0.x release (0.3.0)
• update package engines field to indicate minimum Node.js version consistent with Travis CI update

[kethinov]

Already updated engines field. Altered version to bump minor instead of patch.

[kethinov]

All right, finally got the tests passing. Should be ready for review now.

[brodybits]

A couple more questions:

• Why remove component.json? Any idea what it was used for, and if there is anything else we should be doing these days?
• I am a bit ambivalent about committing package-lock.json. My experience is that this kind of lock file locks into outdated dependencies with possible security issues. I use Yarn more often for projects outside of Cordova, actually do have much better experience with yarn.lock (good for quickly moving between various releases with different versions of package dependencies).

Otherwise LGTM. I wish I had more time to understand these updates in depth, oh well.

[kethinov]

As far as I can tell component.json is a manifest file for an obsolete package manager nobody uses anymore. The official docs refer to it as deprecated.

Regarding package-lock, the recommended practice from npm is to commit it. As I understand it, keeping the dependency out of date is seen as a feature in a manner of speaking. This makes it so npm ci can do reproducible builds that always use the same version of every package and every downstream package as well.

Before package-lock files, every time someone ran npm i, different versions of packages or downstream packages might be installed based on whether or not anything in the tree used *, ^, or ~ in its version list, resulting in working builds for some people and broken builds for other people depending on when they ran npm i. Committing the lockfile to the repo makes sure everyone has the same build every time they run npm ci.

[codler]

Thanks @kethinov for your work on supporting this package, I am sorry that I could not provide very much active support for the recent issues and contributions.

I would personally favor a couple things pretty strongly:

• new 0.x release (0.3.0)
• update package engines field to indicate minimum Node.js version consistent with Travis CI update

Can we change to 1.x release?

[timbru31]

Thanks for the long overdue cleanup! 💯
I'm fine with both a 0.3.0 or a 1.0.0 release as both indicate breaking changes. What's the your argument for a 1.0.0 @codler?

[codler]

Thanks for the long overdue cleanup! 💯
I'm fine with both a 0.3.0 or a 1.0.0 release as both indicate breaking changes. What's the your argument for a 1.0.0 @codler?

When you do npm update later on it will upgrade more correctly.

[timbru31]

Can you elaborate on that? npm update should not upgrade 0.2.x to 0.3.0, because a minor version bump is considered a breaking change if the major version is zero (0).

See https://semver.org/#spec-item-4 and https://docs.npmjs.com/cli-commands/update.html#caret-dependencies-below-100

[codler]

for example, 0.2.0 doesnt update to 0.2.1 while 1.2.0 do update to 1.2.1 in case we make a patch.

[timbru31]

As far, as I read the npm update documentation, 0.2.1 should be installed in case one specifies ~0.2.0 or ^0.2.0:

Then npm update will install dep1@0.4.1, because that is the highest-sorting version that satisfies ^0.4.0 (>= 0.4.0 <0.5.0)

[kethinov]

I'm agnostic on the 1.0.0 vs 0.3.0 decision. Let's let @brodybits cast the deciding vote?

[brodybits]

• I think that adding the files entry implies a few more things that should be done, as I said in a comment on the code.
• I would generally favor splitting the proposed changes into multiple PRs. This case could be an exception, assuming we merge in a single squashed commit.

[brodybits]

At this point I would downvote a 1.0.0 release. There are multiple breaking changes proposed in this PR, and I would rather see xmldom become a little more stable before we make the 1.0.0 release.

I would generally recommend that users do the following to get the latest updates:

• do npm outdated to show which dependencies are outdated
• do npm add xxx@latest on each non-dev dependency to update (and test before committing and pushing the changes upstream) (--save flag was needed in the past, should not be needed any longer)
• do npm add --save-dev xxx@latest on each dev dependency to be updated (and test before pushing)

[codler]

Why wouldn't it be stable when the lib have been around for so long and so many people are using it?

[kethinov]

Regarding the versioning question, I think the tradeoffs work like this:

Argument for 0.3.0: Reflects the fact that we haven't fundamentally altered this codebase. We've cleaned it up a bit, but it's still the beta quality implementation that it was before we hard forked the project. One could say we should reserve 1.0.0 for when we feel the codebase is more mature and stable rather than merely the working but somewhat hacked together quality that it is right now. Basically keeping it 0.x preserves our lack of confidence that the code is hardened and production ready. It slaps a big "use at your own risk" label on the work.

Argument for 1.0.0: This module is used by a very large number of projects. If there were serious flaws in the implementation, we would know by now. Imperfect though it may be, the age of the project and the persistently large user base implies we should just shed our confidence issues and stamp it 1.0.0 as it may have been overdue for being so labeled already.

Personally I lean towards 0.3.0 for the time being, but we should perhaps file an issue to define what it would take to cross the 1.0.0 finish line more rigorously rather than letting gut feelings decide it. Regardless, I don't have a strong opinion on the subject either way, so whatever everyone else thinks I'm cool with.

[brodybits]

LGTM with 2 suggestions and 1 more thing for a followup in issue #29

I will raise the issue to discuss 1.0.0 release.

[brodybits]

Thanks @kethinov!

[angular-git-user]

Can we move this to 1.0.0? It is breaking our application which has a dependency on
"@azure/service-bus": "1.0.1"
We have a Node version requirement of 8.11.0
and we cannot move to 10.X

[kethinov]

Can you post a stack trace?

The problem is likely with some intermediate dependency that is depending on xmldom using too flexible a wildcard, e.g. ^ or *. In that case, a PR should be submitted to that project to anchor the dependency to ~ or a fixed version.

[brodybits]

I think this is something that should be fixed in your dependency (@azure/service-bus": "1.0.1").

npm semver with caret (^) should work like tilde (~) for a dependency that is in a 0.x release (exception for 0.0.x).

To quote some resources from the web:

According to 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.

From https://nodesource.com/blog/semver-tilde-and-caret/#majorzeroandminor10yz0yz0y10:

For versions greater than or equal to 0.1.0, but less than 1.0.0, the caret adopts the same behaviour as a tilde and will allow flexibility in patch versions (only).

For example, ^0.1.3 will permit all versions from 0.1.3 to the next minor, 0.2.0.

I would be willing to consider supporting Node.js 8 for one more minor release in case of sufficient interest. This would need to be discussed in a new issue, not in a closed issue or PR.

[brodybits]

@kethinov you should see that I added a couple milestones:

• 0.4.0 for followup changes that should be done soon
• 1.0.0 to track issues and PRs that I think should be resolved before 1.0.0