Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UPDATE: Dropping Node 6 & 7 support, major build/development environment update #212

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

kwhitley
Copy link
Owner

@kwhitley kwhitley commented May 17, 2020

This is a major (to the codebase) release, dropping support for Node 6 & 7, and updating the build process/environment to the point we can begin the major refactor of v2 (currently near impossible because the library was stuck in such an outdated mode to enable Node 6 (and v4 previously) support.

Changelog

  • Node Versions/Support

    • dropping support for ancient Node 6/7 to allow the codebase to progress to the modern era (dropping support for deprecated v8 will follow)
    • dropping direct testing on Travis for all non LTS Node versions (odd number releases)
    • must pass all tests again in Node 8, 10, 12, 14
  • Restify with Node13+

    • dropping official support for restify with Node 13+. May add it back in as a minor release late if we can restore this combo passing the test suite (disabled for now)_ waiting for response from @svozza to see if we can find the culprit in the failing tests
  • Build

    • use yarn for dev work
    • using rollup for module build to allow modern codebase of the core lib
    • adding yarn-release to streamline NPM releases
    • fully update dev deps
  • Testing

    • migrate to jest (from mocha + chai)
    • break apart monolithic test into smaller subtests
  • Bugfixes & Feature Additions

Future Plans

There have been a lot of great suggestions, PRs, conversation, etc around features and additions for apicache. As of now, several of the outstanding PRs are not passing build and will be closed unless addressed. I'll try to go through each and discuss/review the merits of adding it to the codebase, but the short term priority is getting this repo to a state for a full rewrite - with the goal of making it simpler at its core, easier to work on, and extensible with plugins (e.g. swapping out store mechanisms rather than memory store and redis having such tight couplings to the core lib). I'd also like this to be extensible with koa, and all the even-newer flavors of routers that for whatever reason, still lack intuitive caching.

Shameless Plug for itty-router (serverless router)

Much of this library is made basically irrelevant by the move to serverless (which can be cached on the edge more effectively than on the server)... my current work has been towards simplifying the migration path from microservices (express) to serverless, while still having the elegant interfaces/routing we're used to. With this in mind, check out itty-router on NPM. As I move things to the latest-and-greatest lightning rod that is CloudFlare Workers, I'm battling the lack of documentation and pretty rough route handling of their Workers - itty router has helped make that a breeze to combine the best of both worlds - single serverless functions that can split out to handle multiple dynamic routes (within reason).

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.531% when pulling b4cfebe on update/node-latest into f27cb2b on master.

@kwhitley kwhitley changed the title TLC update for node versions and current breaks UPDATE: Dropping Node 6 & 7 support, major build/development environment update May 17, 2020
@kwhitley
Copy link
Owner Author

@mvasigh tagging you in here for your eyes - step one is leave codebase alone and update scaffolding around it, then once that's sorted, dig into the core for an update.

I think this will warrant a major version release despite no changes to the API simply due tot he dropping of legacy Node versions - thoughts? I hate bumping major versions for an internal update, but...

@mvasigh
Copy link
Collaborator

mvasigh commented May 18, 2020

@mvasigh tagging you in here for your eyes - step one is leave codebase alone and update scaffolding around it, then once that's sorted, dig into the core for an update.

I think this will warrant a major version release despite no changes to the API simply due tot he dropping of legacy Node versions - thoughts? I hate bumping major versions for an internal update, but...

Will the future core updates also include public API changes? If so, maybe this could live on a v2 or next branch once merged and be published under a next tag to eventually evolve into a major version release. If you're not planning any more updates to v1 you could just treat v2 as the default branch. If not though, dropping support for node versions is a totally valid reason for bumping a major version (it's a breaking change) but maybe it's an opportunity for API refinements as well 😁 A pluggable adapter interface with hooks on write/retrieve (to decouple the cache implementation from the middleware) seems like it'd be nice (#173, #207) and the same might apply for creating middleware/integrations (#193) and the package could just ship a simple Express integration built on top of it.

I think it makes total sense to focus on the build/test scaffolding first before touching the internals! Just trying to get a bigger picture of what the goal is long term. FWIW I think all of the above could be done while preserving the existing public API as well by just making additive changes and reimplementing the public API internally.

@kwhitley
Copy link
Owner Author

@mvasigh Totally agree w the dream of rolling in API changes/new interface into the major version w these changes... BUT... I'll need to provide LTS for this existing API for some time to come until people migrate, so I at least need to get existing implementation to a solid/stable state before I go off the on the @next. So with that in mind... break SemVer and stick to a 1.x release for this release and save v2 for a proper API shift, or bump early to a 2.x and save v3.x as the next API? The latter feels dirty, but technically more correct...

@mvasigh
Copy link
Collaborator

mvasigh commented May 18, 2020

semver/semver#468 is interesting, there's discussion around this idea... I think the major version bump is probably the safer option, though I get the argument that it's not really a "breaking public API change." But it could be a breaking change for some environments. I guess I think the technically more correct version makes more sense.

@kwhitley
Copy link
Owner Author

So we continue to be plagued by issues trying to resolve tests using restify with Node v13+ (with or without gzip, I'm finding).

I'm proposing we finish updating the repo with the changes above (dropping old Node support), and temporarily drop official support for restify (specifically when used with node 13+). I'd add in a check to allow Travis to run the tests for restify+gzip with Node 12 and under. Either way, this will be part of the v2 release that will go into cruise control while we make a decent/all-new API for v3.

@svozza as the key stakeholder/contributor for restify in apicache, how do you feel about this? Conversely, if you know of key under-the-hood differences (I think around setting/reading headers) between node 12 --> 13 that would affect restify, let me know!

@svozza
Copy link
Contributor

svozza commented May 20, 2020

Hey Kevin, sorry, I'm only seeing this now. I think this is perfectably reasonable, I'll try and investigate what's going on here and as you say we can just add it back in in a minor version.

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

Successfully merging this pull request may close these issues.

None yet

4 participants