Skip to content

Implement app.defineMiddlewarePhases - #786

Merged
bajtos merged 1 commit into
masterfrom
feature/define-middleware-phases
Nov 12, 2014
Merged

Implement app.defineMiddlewarePhases#786
bajtos merged 1 commit into
masterfrom
feature/define-middleware-phases

Conversation

@bajtos

Copy link
Copy Markdown
Member

Implement method for registering (new) middleware phases.

  • If all names are new, then the phases are added just before
    the "routes" phase.
    • Otherwise the provided list of names is merged with the existing
      phases in such way that the order of phases is preserved.

Example

// built-in phases:
// initial, session, auth, parse, routes, files, final
app.defineMiddlewarePhases('custom');
// new list of phases
// initial, session, auth, parse,
// custom,
// routes, files, final
app.defineMiddlewarePhases([
'initial', 'postinit', 'preauth', 'routes', 'subapps'
]);
// new list of phases
// initial,
// postinit, preauth,
// session, auth, parse, custom,
// routes,
// subapps,
// files, final

A part of #739, requires strongloop/loopback-phase#5

/to @ritch please review
/cc @raymondfeng

@bajtosbajtos mentioned this pull request Nov 11, 2014
8 tasks
@bajtosbajtos added this to the #Epic: Components milestone Nov 11, 2014
Implement method for registering (new) middleware phases.
- If all names are new, then the phases are added just before
the "routes" phase.
- Otherwise the provided list of names is merged with the existing
phases in such way that the order of phases is preserved.
Example
// built-in phases:
// initial, session, auth, parse, routes, files, final
app.defineMiddlewarePhases('custom');
// new list of phases
// initial, session, auth, parse,
// custom,
// routes, files, final
app.defineMiddlewarePhases([
'initial', 'postinit', 'preauth', 'routes', 'subapps'
]);
// new list of phases
// initial,
// postinit, preauth,
// session, auth, parse, custom,
// routes,
// subapps,
// files, final
@bajtos
bajtosforce-pushed the feature/define-middleware-phases branch from 877949e to 98d4390CompareNovember 11, 2014 18:45
Comment threadlib/server-app.js

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be in loopback-phase?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to refactor the code to register phases by relative order to loopback-phase.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I'll land this patch and submit the refactoring in a new PR.

@ritch

Copy link
Copy Markdown
Member

LGTM

@ritchritch assigned bajtos and unassigned ritchNov 11, 2014
bajtos added a commit that referenced this pull request Nov 12, 2014
@bajtos
bajtos merged commit 4474f8b into masterNov 12, 2014
@bajtosbajtos removed the #review label Nov 12, 2014
@bajtos
bajtos deleted the feature/define-middleware-phases branch November 12, 2014 07:16
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@bajtos@ritch@raymondfeng@crandmck