From 23b3b26c067c1b5c02bd9ef3e9e82cf5b8904144 Mon Sep 17 00:00:00 2001 From: Harminder Virk Date: Sun, 16 May 2021 13:45:21 +0530 Subject: [PATCH 1/3] doc: document the RFC process --- README.md | 111 ++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 79 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 7279a53..1826ba3 100644 --- a/README.md +++ b/README.md @@ -1,56 +1,103 @@ # Adonis RFCs -> Give your ideas some life :) +> The process is inspired by the [Vuejs RFC process](https://github.com/vuejs/rfcs). Currently, we are testing out the RFC process and may tweak it as we go along and learn new things. -Many small changes to the API and bug fixes can be done without creating any RFC. However, we may ask you to create a formal RFC for changes of the following nature. +Many small changes to the existing APIs and bug fixes are usually done without creating any RFC. However, we may ask you to create a formal RFC for modifications of the following nature. -- Adding an entirely new API or module to the eco-system. +- Adding an entirely new API or module to the ecosystem. - Changing the behavior of an existing feature. -- Features that are not clear and requires more discussion and community involvement. +- Features that are not clear and require more discussion and community involvement. -## How to know if RFC is required? +## Why do you need to do this? -> Feel free to join the [discord server](https://discordapp.com/invite/vDcEjq6) and ask it on general channel. If you think, you need more guidance, then feel free to ping `@virk` on Discord. +We appreciate you taking out the time and sharing your ideas to improve the framework. However, as AdonisJS is growing and getting used in production, we have to be more responsible and careful with the changes or features we add to the framework. -RFC is required, when the feature cannot be implemented separately and should be added to the core or the eco-system of first party plugins. +Also, adding a new feature to the framework core or the official set of packages means creating more work for the framework creator/core team. We may not always be interested in executing new ideas or maintaining them in the long term, as there is always some ongoing work. -If you think, a feature can be added and maintained as a 3rd party add-on, then there is no need of RFC. Just work on the feature, let us know, and we will be happy to spread the word. +With that said, we still encourage you to share your ideas with us and see if we can bring them together to life. -## Getting started +## Taking the process seriously -Adding a new feature, or changing behavior of an existing feature is a big responsibility. Make sure to spec out the feature clearly and explain clear use cases. +The RFC process is not only about sharing your wishlist of features. You should do some proper research and create an actionable RFC. -A general template for creating an RFC can be. +#### ❌ Bad example of an RFC -1. Define the problem you are trying to solve. -2. A bigger picture of the feature and how it will work as a concept. -3. Technical details +In the following example, you are transferring the weight of research and finalizing the implementation level details. ** These kinds of RFCs will be closed without any explanation**. -Another important aspect is to make sure that the feature is within the scope of [AdonisJs philosophy](). +``` +Please add support for filesystem-based routing. It will be helpful for me and others as well. -## Creating your first RFC +These are some frameworks that do it. +``` -Let's get started together and create your first RFC. +#### ✅ A better example of an RFC -1. Click [here](https://github.com/adonisjs/rfcs/issues/new) to open a new issue. -2. Add the title of the issue. For example **Adding GraphQL as the first-class citizen to AdonisJs**. -3. Fill out the issue template. If you think any section is irrelevant, write `NA.` -4. Double check the issue and submit it. -5. Normally someone from the core team will give appropriate labels to the issue and get in touch with you in a couple of days. -6. If you think your RFC is waiting for the response for a more than 7days, please feel free to reach `@virk` on Discord server, or email `virk[at]adonisjs[dot]com.` +``` +- Share the feature you want the framework to have +- Share the basic usage examples +- Share if there are going to be any performance issues by introducing the feature +- If a variation of that API already exists, then have a conversation with the core team and try to understand why they opted for the other variation +- Share design and implementation details +- Share unknows - We will be more than happy to discuss them in-depth with you +- Share know limitations +``` -## Lifecycle +## What the process is +In short, to get a major feature added to AdonisJS, one must first get the +RFC merged into the RFC repo as a markdown file. At that point, the RFC +is 'active' and may be implemented to achieve eventual inclusion +into AdonisJS. -Following is the lifecycle in which features are picked, developed and shipped to production. +* Fork the RFC repo https://github.com/adonisjs/rfcs - +* Copy `0000-template.md` to `active-rfcs/0000-my-feature.md` (where +'my-feature' is descriptive. Don't assign an RFC number yet). -## Filtering issues +* Fill in the RFC. Put care into the details: **RFCs that do not +present convincing motivation, demonstrate understanding of the +impact of the design, or are disingenuous about the drawbacks or +alternatives tend to be poorly received**. -Before starting your own RFC, it is important to look for the existing one's and contribute to them. +* Submit a pull request. Make sure to follow the pull request template and open a corresponding discussion thread. -You can filter the features by their category using the `scope:` label. +* Build consensus and integrate feedback in the discussion thread. RFCs with broad support are much more likely to make progress than those who don't receive any comments. -For example: The RFC for changes in the HTTP layer will have the label of `scope:HTTP`, for Database `scope:Database` and so on. +* Eventually, the [core team] will decide whether the RFC is a candidate +for inclusion in AdonisJS. + +* An RFC can be modified based upon feedback from the [core team] and community. Significant modifications may trigger a new final comment period. + +* An RFC may be rejected after the public discussion has settled +and comments have been made summarizing the rationale for rejection. A member of the [core team] should then close the RFC's associated pull request. + +* An RFC may be accepted at the close of its final comment period. A [core team] member will merge the RFC's associated pull request, at which point the RFC will become 'active'. + +## Details on Active RFCs + +Once an RFC becomes active, then authors may implement it and submit the +feature as a pull request to the applicable AdonisJS repo. Becoming 'active' is not a rubber stamp, and in particular, it still does not mean we will ultimately merge the feature; it does mean that the core team has agreed to it in principle and are amenable to merging it. + +Furthermore, the fact that a given RFC has been accepted and 'active' implies nothing about what priority is assigned to its implementation or whether anybody is currently working on it. + +We can do modifications to active RFCs in follow-up PRs. We strive +to write each RFC in a manner that will reflect the final design of +the feature; but the nature of the process means that we cannot expect +every merged RFC to reflect what the end result will be at +the time of the next major release; therefore, we try to keep each RFC +document somewhat in sync with the language feature as planned, +tracking such changes via follow-up pull requests to the document. + +## Implementing an RFC + +The author of an RFC is not obligated to implement it. Of course, the RFC author (like any other developer) is welcome to post an implementation for review after the RFC has been accepted. + +An active RFC should have the link to the implementation PR listed if there is one. You should conduct feedback on the actual implementation in the implementation PR instead of the original RFC PR. + +If you are interested in working on the implementation for an 'active' +RFC, but cannot determine if someone else is already working on it, +feel free to ask (e.g., by leaving a comment on the associated issue). + +## Reviewing RFC's + +Members of the [core team] will attempt to review some set of open RFC +pull requests regularly. If a core team member believes an RFC PR is ready to be accepted into active status, they can approve the PR using GitHub's review feature to signal their approval of the RFC. -## Coding style -Make sure to read the [contributing guidelines](https://adonisjs.com/docs/4.1/contribution-guide), before getting started. From 08626dc0f3b284608c319d90359380c63ef504e8 Mon Sep 17 00:00:00 2001 From: Harminder Virk Date: Sun, 16 May 2021 13:58:19 +0530 Subject: [PATCH 2/3] docs: make a few adjustments --- README.md | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 1826ba3..81d1211 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,9 @@ The RFC process is not only about sharing your wishlist of features. You should #### ❌ Bad example of an RFC -In the following example, you are transferring the weight of research and finalizing the implementation level details. ** These kinds of RFCs will be closed without any explanation**. +In the following example, you are transferring the weight of research and finalizing the implementation level details on the core team. + +**These kinds of RFCs will be closed without any explanation**. ``` Please add support for filesystem-based routing. It will be helpful for me and others as well. @@ -31,7 +33,6 @@ These are some frameworks that do it. #### ✅ A better example of an RFC -``` - Share the feature you want the framework to have - Share the basic usage examples - Share if there are going to be any performance issues by introducing the feature @@ -39,12 +40,11 @@ These are some frameworks that do it. - Share design and implementation details - Share unknows - We will be more than happy to discuss them in-depth with you - Share know limitations -``` ## What the process is In short, to get a major feature added to AdonisJS, one must first get the RFC merged into the RFC repo as a markdown file. At that point, the RFC -is 'active' and may be implemented to achieve eventual inclusion +is `'active'` and may be implemented to achieve eventual inclusion into AdonisJS. * Fork the RFC repo https://github.com/adonisjs/rfcs @@ -57,24 +57,26 @@ present convincing motivation, demonstrate understanding of the impact of the design, or are disingenuous about the drawbacks or alternatives tend to be poorly received**. -* Submit a pull request. Make sure to follow the pull request template and open a corresponding discussion thread. +* Submit a pull request. Make sure to follow the pull request template and open a corresponding [discussion thread](https://github.com/thetutlage/rfcs/discussions). * Build consensus and integrate feedback in the discussion thread. RFCs with broad support are much more likely to make progress than those who don't receive any comments. -* Eventually, the [core team] will decide whether the RFC is a candidate +* Eventually, the [core team](https://github.com/orgs/adonisjs/people) will decide whether the RFC is a candidate for inclusion in AdonisJS. -* An RFC can be modified based upon feedback from the [core team] and community. Significant modifications may trigger a new final comment period. +* An RFC can be modified based upon feedback from the [core team](https://github.com/orgs/adonisjs/people) and community. Significant modifications may trigger a new final comment period. * An RFC may be rejected after the public discussion has settled -and comments have been made summarizing the rationale for rejection. A member of the [core team] should then close the RFC's associated pull request. +and comments have been made summarizing the rationale for rejection. A member of the [core team](https://github.com/orgs/adonisjs/people) should then close the RFC's associated pull request. + +* An RFC may be accepted at the close of its final comment period. A [core team](https://github.com/orgs/adonisjs/people) member will merge the RFC's associated pull request, at which point the RFC will become 'active'. -* An RFC may be accepted at the close of its final comment period. A [core team] member will merge the RFC's associated pull request, at which point the RFC will become 'active'. +* RFCs created by the core team may not wait for the consensus, since the work implemented in the RFC is directly maintained by the core team. However, if there is a strong push back, then the core team must build a consensus or drop the idea. ## Details on Active RFCs Once an RFC becomes active, then authors may implement it and submit the -feature as a pull request to the applicable AdonisJS repo. Becoming 'active' is not a rubber stamp, and in particular, it still does not mean we will ultimately merge the feature; it does mean that the core team has agreed to it in principle and are amenable to merging it. +feature as a pull request to the applicable AdonisJS repo. Becoming `'active'` is not a rubber stamp, and in particular, it still does not mean we will ultimately merge the feature; it does mean that the core team has agreed to it in principle and are amenable to merging it. Furthermore, the fact that a given RFC has been accepted and 'active' implies nothing about what priority is assigned to its implementation or whether anybody is currently working on it. @@ -98,6 +100,6 @@ feel free to ask (e.g., by leaving a comment on the associated issue). ## Reviewing RFC's -Members of the [core team] will attempt to review some set of open RFC +Members of the [core team](https://github.com/orgs/adonisjs/people) will attempt to review some set of open RFC pull requests regularly. If a core team member believes an RFC PR is ready to be accepted into active status, they can approve the PR using GitHub's review feature to signal their approval of the RFC. From 92313b35063c3fa51b2bb139790133462bd48f3a Mon Sep 17 00:00:00 2001 From: Harminder Virk Date: Mon, 17 May 2021 10:39:58 +0530 Subject: [PATCH 3/3] feat: add websockets rfc --- active-rfcs/0000-websockets.md | 191 +++++++++++++++++++++++++++++++++ 1 file changed, 191 insertions(+) create mode 100644 active-rfcs/0000-websockets.md diff --git a/active-rfcs/0000-websockets.md b/active-rfcs/0000-websockets.md new file mode 100644 index 0000000..cd90478 --- /dev/null +++ b/active-rfcs/0000-websockets.md @@ -0,0 +1,191 @@ +- Start Date: 2021-05-17 +- Reference Issues: N/A +- Implementation: N/A + +## Summary + +The RFC introduces the support for WebSockets to the framework as a first-class citizen. + +Unlike v4, we **should NOT** roll out our implementation of WebSockets, as it is practically impossible to create and maintain the clients for different platforms. + +Instead, we should make use of `socket.io`. Socket.io has a [broad range of clients](https://socket.io/docs/v4) and v4 also allows creating [`websocket` only](https://socket.io/docs/v4/client-initialization/#transports) connections. + +## Motivation + +Node.js is well known for its real-time capabilities, and being the mainstream framework, we should offer a delightful experience when creating real-time services on top of WebSockets. + +## Basic usage + +The code will be published as a separate official package (`@adonisjs/ws`), and one can import the initialized WebSocket server from the container as follows: + +```ts +import Ws from '@ioc:Adonis/Addons/Ws' + +Ws.on('message', () => { +}) + +Ws.on('typing', () => { +}) +``` + +Define events inside a namespace + +```ts +import Ws from '@ioc:Adonis/Addons/Ws' + +Ws + .namespace('chat') + .on('message', () => { + }) +``` + +Use middleware + +```ts +import Ws from '@ioc:Adonis/Addons/Ws' + +Ws + .on('message', () => { + }) + .middleware('auth') +``` + +## Detailed Design + +The WebSocket support should follow/use the existing patterns used by the framework and hide most of the complexities and boilerplate. + +Since we are using `socket.io`, it will do the heavy lifting for managing rooms/channels and multiplexing. We just need to take care of the following: + +- Allow using Controllers to handle events. Just like routes and HTTP controllers. +- Allow using Middleware just like the HTTP requests. +- Support for all the socket.io features. + +### Socket context + +Just like the [HTTP context](https://docs.adonisjs.com/guides/context), we will create a context instance for the WebSocket connections as well. It will have more or less the same properties alongside the [socket instance](https://socket.io/docs/v4/server-socket-instance/). + +### Event listeners + +Socket.io make you to register the event listeners on every socket instance. For example: + +```ts +io.on('connection', (socket) => { + socket.on('message', () => {}) +}) +``` + +I want to move the event handlers at the top level and internally use [socket.onAny](https://socket.io/docs/v4/listening-to-events/#socket-onAny-listener) to invoke event handlers defined on the AdonisJS specific `Ws` object. + +```ts +Ws.on('message', () => {}) +``` + +Also, one will receive the Websocket context as the first argument and the event data as the 2nd argument. + +```ts +Ws.on('message', (ctx, data) => { + ctx.socket + ctx.auth.user + ctx.request +}) +``` + +### Controllers +Just like the HTTP routes, one should be able to bind a controller method as the event handler. For example: + +```ts +import Ws from '@ioc:Adonis/Addons/Ws' + +Ws.on('message', 'ChatController.handleMessage') +``` + +And define the controllers as follows: + +```ts +export default class ChatController { + public async handleMessage (ctx, data) { + } +} +``` + +### Working with namespaces +Socket.io makes use of the `of` method to create a [namespace](https://socket.io/docs/v4/namespaces/). I personally try to keep naming more descriptive and will opt for the `namespace` method. + +```ts +Ws.namespace('chat') +``` + +Optionally one can register a connection handler as follows. The connection runs for every new socket connection after all the middleware. + +```ts +Ws + .namespace('chat') + .connected(() => {}) +``` + +Similarly, a `disconnected` handler is invoked when a socket disconnects. This is similar to the `socket.on('disconnect')` event of socket.io. + +```ts +Ws + .namespace('chat') + .disconnected(() => {}) +``` + +#### Dynamic namespaces + +Namespaces in socketio can be [dynamic](https://socket.io/docs/v4/namespaces/#Dynamic-namespaces) using a regular expression or a function. I believe we can make it work more like HTTP routes. + +**I have not explored the technical possibilities here, but it should be doable to have routes like params in ws namespaces.** + +```ts +Ws.namespace('/chat/:channel') +``` + +### Middleware +Just like HTTP middleware, one should be able to define middleware for all or a specific namespace. + +Middleware registered globally will be applied to all the namespaces. + +```ts +Ws.middleware.register([ + () => import('App/Middleware/Name') +]) +``` + +Named middleware can be referenced on a specific namespace. + +```ts +Ws.middleware.registerNamed({ + auth: () => import('App/Middleware/Auth') +}) + +Ws + .namespace('chat') + .middleware('auth') +``` + +### Rooms +Just like socket.io, only the server can make a socket join a given room. For example: + +```ts +Ws.on('connection', ({ socket }) => { + socket.join('room name') +}) +``` + +Socketio doesn't have an API for clients to request for joining a room. It is always the server that adds a socket to a room. + +### Broadcasting API +The broadcasting API will be same as [socket.io](https://socket.io/docs/v4/emit-cheatsheet/). + +### CORS +Socket.io has its configuration for CORS. In our case, we can inherit from the CORS config used by the HTTP server but still allowing a separate CORS config for the WebSocket server. + +### Clients +One can continue using the existing Socket.io clients. Nothing needs to change for them to work with AdonisJS. + +## Known limitations +NONE + +## Breaking change adoption strategy +NONE