taoqf / node-html-parser

A very fast HTML parser, generating a simplified DOM, with basic element query support.
MIT License
1.11k stars 107 forks source link

semver and breaking changes #87

Closed moltar closed 3 years ago

moltar commented 3 years ago

Not sure if this package is following SemVer, but there was a breaking change in options, which did not bump the major.

https://github.com/taoqf/node-html-parser/commit/d7a8c292cf81041053ebdcdc7535d084af6c2f89

Thanks.

taoqf commented 3 years ago

It must be, the default value of script, style and 'pre' changed to true. Then, what should I do now? unpublish v1.3.3?

moltar commented 3 years ago

Hm, previously I was using it like this:

parse(html, { script: true })

And now it's like this:

parse(html, {
    blockTextElements: {
      script: true,
    },
  })

So it seems it is more than just a defaults change.

taoqf commented 3 years ago

Sorry I didn't get your point, What is the issue now?

moltar commented 3 years ago

No issue now, as I have already migrated.

But the issue was this.

When I was using 1.3.0, the options were:

parse(html, {
  script: true
})

And when I upgraded to 1.4.9, the options are:

parse(html, {
    blockTextElements: {
      script: true,
    },
  })

That was a breaking change, but the major version was not bumped.

taoqf commented 3 years ago

You mean I should set the major version 2?

moltar commented 3 years ago

If you are following SemVer, then yes.

taoqf commented 3 years ago

Ah, What should I do now? could you please help me.

moltar commented 3 years ago

Well, I think it is too late now. But you can consider that in the future.

taoqf commented 3 years ago

OK, thank you very much.

moltar commented 3 years ago

Here you can read about SemVer: https://semver.org/

This is not a requirement, but most NPM packages follow that.

ajimix commented 3 years ago

Yes please, or at least add CHANGELOG.md with changes so it's easier to upgrade without breaking stuff

taoqf commented 3 years ago

Thank you all, I will do it after 2.0