elysia icon indicating copy to clipboard operation
elysia copied to clipboard

Elysia 1.0 Breaking Changes / Migration

Open SaltyAom opened this issue 4 months ago • 14 comments

Breaking change

It's sad but the upcoming Elysia 1.0 RC will contain breaking changes for all users, especially library authors :aris_crying:

Lifecycle event register with "on" will now only apply to the current and descendant instances.

Migration

Migration should take no longer than 10-15 minutes. But for production apps, it heavily depends on library authors.

Starting from Elysia 1.0 RC, all lifecycle events including derive, and resolve will accept additional parameters { as: 'global' | 'local' } to specify if hook should be local or global which default to local

To migrate, add { as: 'global' } to any even register using .on to specify it as global.

// From Elysia 0.8
new Elysia()
    .onBeforeHandle(() => "A")
    .derive(() => {})

// Into Elysia 1.0
new Elysia()
    .onBeforeHandle({ as: 'global' }, () => "A")
    .derive({ as: 'global' }, () => {})

Behavior

If { as: 'global' } is not added, a parent instance that uses the current instance will not inherits a hook as the following:

const plugin = new Elysia()
    .onBeforeHandle(() => {
        console.log('Hi')
    })
    // log Hi
    .get('/hi', () => 'h')

const app = new Elysia()
    .use(plugin)
    // no log
    .get('/no-hi', () => 'h')

If is a behavior change from 0.8.

  • 0.8: Both /hi and /no-hi will log
  • 1.0: Only /hi will log

As the plugin hook now only defaults to itself and its descendant, it will not be applied to the parent (main).

To make it apply to the parent, explicit annotation is needed.

const plugin = new Elysia()
    .onBeforeHandle({ as: 'global' }, () => {
        console.log('Hi')
    })
    // log Hi
    .get('/hi', () => 'h')

const app = new Elysia()
    .use(plugin)
    // no log
    .get('/no-hi', () => 'h')

This will mark a hook as global, which is a default behavior of 0.8.

API Affected

To help with this, you can usually find a global hook by using find all with .on. For derive, mapDerive, resolve, mapResolve will show a type warning as missing if use outside of the scope.

The following API or need to be migrate:

  • parse
  • transform
  • beforeHandle
  • afterHandle
  • mapResponse
  • onError
  • onResponse
  • trace
  • derive
  • mapDerive
  • resolve
  • mapResolve

API that does NOT need to be migrated:

  • request

Why

We know we don't like migration as well, but we think this is an important breaking change that will happen sooner or later to fix current problems:

  • We found that many developers have a lot of nested guard even on the new instance. Guard is now almost used as a way to start a new instance that will not affect the main one. To fix this, we default the hook to local by default, removing the boilerplate need.
  • We believe that every hook global by default can cause unpredictable behavior if not careful, especially in a team with inexperienced developers. We asked many developers and found that many expected the hook to be local at first without reading the documentation.
  • Following the previous point, we found that making hook global by default can easily cause accidental bugs if not reviewed carefully, and hard to debug as of observability is hard to distinguish by eyes especially in large PR and codebase. We want to reduce the said user-error as much as possible.

Once migrated, your API should have an easier mental model, be more predictable, and be easier to maintain. For example, see the below picture which does the same thing.

Screenshot 2567-03-01 at 19 48 07

For additional information, please see https://x.com/saltyAom/status/1763554701130514944?s=20

SaltyAom avatar Mar 01 '24 13:03 SaltyAom