fix(package): add @octokit/core as peer dependency

[gr2m]

fixes #124

[gr2m]

@AArnott you can test this PR by doing the yarn counterpart of npm install https://github.pika.dev/octokit/plugin-paginate-rest.js/pr/130

[AArnott]

Yes, I'll test this out and if it doesn't work I'll give you repro steps. My GitHub username ends with two "t"s by the way, so you mentioned someone else.

[AArnott]

Based on #124 (comment) and my local testing of the workaround, it appears this fix will work.

[github-actions]

🎉 This PR is included in version 2.3.1 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[timocov]

This fix just dropped support for packages/libraries with TypeScript below 3.8 version... Is there a difference between using import and import type here? Why can't we use just import instead?

[timocov]

@gr2m I mean, this was released as a patch release, but contains breaking changes. Is that okay?

[gr2m]

My understanding is that import type will make sure that no code will be imported into the final code. I don't consider TypeScript changes breaking changes because you don't use it in production, only in development. TypeScript itself does not follow semver with its releases.

Is there a reason you are stuck with TypeScript below 3.8?

[timocov]

Is there a reason you are stuck with TypeScript below 3.8?

Some of our branches are still on 3.7. Is that really problem and too outdated? I don't think so. It's just around 1yo version though 😂

My understanding is that import type will make sure that no code will be imported into the final code

The same as import if you'll use imported reference as type and in types only. So if you don't use it anywhere else from types, they are completely the same. But in d.ts files (which you provide with the npm package to consumers) they are always the same, because d.ts is used for the type checking stage only (d.ts files doesn't produce any output).

I don't consider TypeScript changes breaking changes because you don't use it in production, only in development

But this changes might accidentally break any build (development as well) without any reason, just because there is the new patch version of the library, and if you don't use lock files (there might be some reasons why you don't/can't use them) you'll have "happy morning" by fixing compilation errors, even if you didn't do any change in your code.

TypeScript itself does not follow semver with its releases.

And you? I don't think that versioning of this package is related to TypeScript's versioning system.

This looks similar to if you published the new boost/Qt patch version of the library and have started to use new features from the new C++ standard. I believe in this case it should be the new major version at least to avoid confusing and inconvenience.

[timocov]

@gr2m I don't say that you don't need to use features from the new TypeScript version, but if someone has consumers, they should worry about it. In this case, my personal opinion that using import type isn't necessary, but if you decided to use it - please bump a major version instead to avoid breaking code. Or declare that all libraries doesn't follow semver (but I guess it requires more changes, because they need to change dependencies and devDependencies lists). Please don't get me wrong.

[gr2m]

can you send a PR to change the import type to just import?

[timocov]

Sure #160