Skip to content

Drop node v12 support #4279

Closed
Closed

Description

@kanongil

Support plan

  • is this issue currently blocking your project? (yes/no): no
  • is this issue affecting a production system? (yes/no): no

Context

  • node version: n/a
  • module version: v20.x
  • environment (e.g. node, browser, native): node
  • used with (e.g. hapi application, another framework, standalone, ...): standalone
  • any other relevant information:

What problem are you trying to solve?

Allow to develop hapi and modules using more current node.js and javascript APIs and features. v12 is in maintenance mode and support ends 2022-04-30.

New JS features we can use:

  1. Optional chaining syntax (?.).
  2. Nullish coalescing operator (a ?? b).
  3. Private class methods (this.#privateMethod()).
  4. WeakRef support.

From node 14 (LTS with v14.15.0) we get:

  1. ESM modules (stable from v14.17.0, but also in v12.22.0).
  2. Top-Level await support.
  3. A more stable stream.pipeline().
  4. Fs.rm() method.

If we require v14.17+, we could also sneak in:

  1. AbortSignal support in node APIs.

Do you have a new or modified API suggestion to solve the problem?

N/A.

Activity

  1. added
    featureNew functionality or improvement
    breaking changesChange that can breaking existing code
    on Aug 27, 2021
  2. kanongil commented on Aug 28, 2021

    @kanongil
    ContributorAuthor

    So I haven't given much credence to this, beyond listing some of the stuff we could possibly use with this. I will try to find some concrete examples of how this can make Hapi better.

    The first is that "Top-Level await" will enable simpler code examples in docs and tutorials, as any time the framework needs to be initialized, a wrapper method is currently used to encapsulate the async logic. Eg. the example function here:

    hapi/API.md

    Lines 1365 to 1390 in c2107e9

    const Hapi = require('@hapi/hapi');
    async function example() {
    const server = Hapi.server({ host: 'localhost', port: 8000 });
    // Defines new handler for routes on this server
    const handler = function (route, options) {
    return function (request, h) {
    return 'new handler: ' + options.msg;
    }
    };
    server.decorate('handler', 'test', handler);
    server.route({
    method: 'GET',
    path: '/',
    handler: { test: { msg: 'test' } }
    });
    await server.start();
    }

    With top-Level await, this wrapper can be removed, making the code easier to follow.

    Feel free to come up with other examples where the new features can help.

  3. kanongil commented on Aug 28, 2021

    @kanongil
    ContributorAuthor

    "Optional chaining" can simplify existing code wherever we use the pattern object && object.property. This can in most cases be rewritten as the more succinct object?.property. Eg. here:

    const plugin = realm && realm.plugin;

  4. kanongil commented on Aug 28, 2021

    @kanongil
    ContributorAuthor

    "Private class methods" allows us to hide methods from public objects in a simple to use manner.

    This means we can better control the public API and plugins can no longer use such private APIs. Eg. we don't risk the private response._header() being called outside of our control.

  5. kanongil commented on Aug 28, 2021

    @kanongil
    ContributorAuthor

    "Nullish coalescing" is especially useful when extracting default values from passed options and can often work better than the || operator that is frequently used for this.

    Eg. here is an obvious example, where it would actually fix a weird inconsistency when the timeout is 0:

    options.timeout = options.timeout || 5000; // Default timeout to 5 seconds

    =>

    options.timeout = options.timeout ?? 5000;

    But this is a tricky change, as the "" and false values would no longer use the default value, but be passed on as-is. This should not be a problem when called from Typescript, but regular JS user might (inadvertently) rely on it.

    Btw, the above can be even better written as this, where it would completely skip the assignment when there is a value supplied:

    options.timeout ??= 5000;

    This is a minor optimization, and potentially avoids a thrown exception when the supplied value is not writable.

    Edit: The ??= syntax requires node v16.

  6. kanongil commented on Aug 28, 2021

    @kanongil
    ContributorAuthor

    stream.pipeline() could probably be used to simplify the custom pipelining code here:

    hapi/lib/transmit.js

    Lines 359 to 377 in c2107e9

    internals.chain = function (sources) {
    let from = sources[0];
    for (let i = 1; i < sources.length; ++i) {
    const to = sources[i];
    if (to) {
    from.on('error', internals.errorPipe.bind(from, to));
    from = from.pipe(to);
    }
    }
    return from;
    };
    internals.errorPipe = function (to, err) {
    to.emit('error', err);
    };

    Edit: while it can make the code simpler, it will also require extra processing since stream.pipeline() is very complex and support lots of use-cases.

  7. Nargonath commented on Aug 28, 2021

    @Nargonath
    Contributor

    Thanks for the examples Gil.

    I believe Eran already used private class methods somewhere in the org already.

  8. kanongil commented on Aug 28, 2021

    @kanongil
    ContributorAuthor

    I believe Eran already used private class methods somewhere in the org already.

    No, only private instance fields, which is supported in node 12 but was still not fully part of ES spec when we started using it.

    Btw, I would guess that private class methods can allow (slightly) more efficient interpretation / compiled code, since V8 can rely on that the methods never changes.

  9. Nargonath commented on Aug 30, 2021

    @Nargonath
    Contributor

    No, only private instance fields, which is supported in node 12 but was still not fully part of ES spec when we started using it.

    Yeah you're right, my bad. I read your initial comment too fast. 😅

    Btw, I would guess that private class methods can allow (slightly) more efficient interpretation / compiled code, since V8 can rely on that the methods never changes.

    That would make sense indeed.

  10. kanongil commented on Sep 20, 2021

    @kanongil
    ContributorAuthor

    I added AbortSignal to the list. It seems like a promising way to simplify / unify aborting async code.

    This is not only interesting for simplifying internal processing, but also to extend the hapi API itself. Eg. h could include a new aborted property, which a handler can pass along to node API calls:

    handler(request, h) {
    
        return Util.promisify(Fs.readFile('/some/file', { signal: h.aborted }));
    }

    Then if a client closes the request before the payload has been fully delivered, hapi can trigger the h.aborted signal, and automatically cause the read to immediately stop (rather than pointlessly continuing to completion).

  11. devinivy commented on Sep 20, 2021

    @devinivy
    Member

    Very cool, thanks for surfacing this.

  12. devinivy commented on Oct 8, 2021

    @devinivy
    Member

    Another one: we should move to checking res.writableEnded rather than res.finished.

  13. kanongil commented on Oct 8, 2021

    @kanongil
    ContributorAuthor

    I did not mention ESM modules. While I think hapi should definitely support these (for user plugins, when imported, and for testing in Lab), I don't think there is an advantage to convert all the implementation to use it.

    The place where it makes most sense, is for modules that are likely to be used browsers. It would enable eg. much of Hoek to be imported directly without transpiling.

    We should however consider how to best interoperate with it. Eg. by removing default exports in public API's.

  14. devinivy commented on Oct 8, 2021

    @devinivy
    Member

    I believe ESM may merit a thread of its own. But I personally would like to see hapi provide proper ESM support, and also would like to see it remain written in CJS.

  15. kanongil commented on Oct 8, 2021

    @kanongil
    ContributorAuthor

    So how to tackle this?

    Most of this can be converted internally without any user-facing changes and essentially only require an updated minimum version of node. Then the actual implementation can be sorted along the way.

    There are however a few of the features that could require breaking changes to implement, and would therefore best be included in the release that changes to require node 14. These are:

    • ESM (revising exports from public APIs).
    • Private class methods (technically it shouldn't, but there are likely code that depends on the current semi-private methods).
    • AbortSignal support (depending on whether it makes sense to break existing APIs).

    As such, any work on this should probably focus on the above.

  16. devinivy commented on Nov 7, 2022

    @devinivy
    Member

    Resolved with v21 #4386

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    breaking changesChange that can breaking existing codefeatureNew functionality or improvement

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions