-
Notifications
You must be signed in to change notification settings - Fork 662
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
New project structure: Separate WebClient, RTMClient packages; monorepo #677
Comments
I love this idea! I've thought it over a couple times in the past (see #330 and #410). In my time thinking, here's some possibly useful conclusions I've drawn:
Working with lernaFirst, a practical description, because it took me (personally) a longtime to grasp what it actually does. Lerna essentially boils down to a cli app that helps developers do a couple of tasks across a bunch of packages in a single monorepo:
What lerna does not do is provide or facilitate similar package structures. This means lerna doesn't help reduce the TypeScript boilerplate required across the packages in this monorepo. The typical response to this is to have a root-level But what if you add another tool, and another tool? You can easily find yourself having configuration files for TypeScript, TSLint, Prettier, Testing Coverage, and more, in each package. This might be feasible for the mere six or seven packages outlined in this issue, but it becomes a problem when we want to add new packages. I can definitely see myself asking "did I forget any config files?" if I were to ever add a package to a monorepo like this. This is also somewhat in contrast with goal 2.
|
Difference | Interactive Messages API | Events API |
---|---|---|
HTTP Response | 200 OK , but allows: in-conversation messages, ephemeral messages, no message |
200 OK with a challenge sometimes. No response message, not even ephemeral messages. |
Action Response | response_url or Web API |
Only Web API |
I feel like the name for the merging of these two packages mostly depends on what the API that respects the differences of the APIs while still managing their common behavior looks like in practice. I was thinking @slack/incoming
, but that's likely to be confused with incoming webhooks. Maybe just @slack/middleware
?
Going forward
Some other topics that came to my mind:
- What a single package actually looks like, i.e. the small details like testing alongside source code or in a separate directory
- Documentation
- It's given that there'll still be doc comments in source files
- Maybe docs generated from source code comments should be centralized in one ultimate resource, similar to how different Slack APIs (e.g. RTM API, Web API, Events API, etc.) are all centralized on https://api.slack.com/
- Individual packages should have guides with their APIs, similar to how the Events API has a guide before it lists the events
- ???
- Profit
All in all, I feel like this is definitely the right direction for this project! I was previously hoping to propose something like this during my summer internship, but now I'm hoping to get to work on it 😄.
thanks for your thoughts @clavin! i'm glad to see that we've got a similar vision. i wanted to respond to some of your comments directly, so here we go. Thanks for doing the research and sharing what you learned about... lerna.
I think this is true, but in many cases a single configuration is all that we will need. For example, if we standardize on TSLint for linting across each of the packages, then a single I'm being optimistic, but I think all
Points taken. My thought process behind that name is that while the For now, I'll leave this point open for discussion. We don't have a strong vision for the future roadmap for those packages, so I'd prefer to make as little change as possible until we know its benefits. I see benefit in factoring out the
I understand this rationale, and if I thought there was a need for distributing the collection of classes as one package, I would agree! But actually, I'd rather push developers to install only the bits they need to use. We should make that the happy path, and make that easier (lower bundle size, cold start time). This also has the benefit of getting more granular stats from npm (if you haven't observed the rise in downloads, its pretty amazing) - which helps maintainers understand the impact of changes we make and overall trends for our users without putting any "phone home" code in the runtime. PS. I do however think there are opportunities in grouping functionality if it wasn't simply a collection of classes, but instead a higher-level framework with opinions and a more complete solution out of the box. But these packages are destined to fill a different purpose: relatively low-level utilities, with broad application (by keeping controversial opinions to a minimum). PPS. There's plenty of interesting ideas like ☝️ and more, we won't run out before the summer 😉 . |
Looks like the TypeScript project will be contributing to ESLint, and plan for it to supercede TSLint. (see "Linting" header microsoft/TypeScript#29288) |
Your distinction between client & server sound a lot like the Features section of the current readme, which (now that I look at it) also already uses the terminology "incoming" and "outgoing". 🤷♀️ But I digress, it doesn't quite feel right as a package name. Otherwise, good points, notes taken! 👍 One last thing I totally forgot about: versioning! Would the monorepo use fixed versioning or independent versioning? I personally like the idea of fixed versioning, and would be happy to see it here; however, I haven't quite yet thought out the pros and cons of both. |
Great new npm feature that we should take advantage of: https://github.com/npm/rfcs/blob/latest/implemented/0010-monorepo-subdirectory-declaration.md |
Description
The project maintainers would like to propose a new structure that would affect this package, the
@slack/interactive-message
package, and the@slack/events-api
package.Goals
Make fewer destinations for people who use these packages to reach the rest of the community and the maintainers. We've noticed that these packages are often used together (as we intended). But if you run into an issue and need some help, it can be needlessly confusing and sometimes impossible for you to understand where the best place is to post an issue.
Reduce build, test, and documentation tooling cost. Each of the packages have their own build system, documentation generation, testing tooling baked into the repo. A large portion of this is duplicate effort to maintain. This cost seems to be growing as the community has made it clear we need type definitions for both
@slack/events-api
and@slack/interactive-messages
(e.g. Add typescript definition file for public API node-slack-events-api#52). The documentation and testing tooling has varying levels of completeness, and we should make it easier for all packages to improve.Minimize changes for existing users. To the extent that its possible, any changes should allow existing users to continue using these packages without needing to change their code. We want existing users to continue to take advantage of updates and advancements.
Foundation for shared abstractions and code reuse. There are various data structures and code that are duplicated in several places, and we'd like to create a structure that makes it possible to consolidate those. For example, the request signing code for the
@slack/events-api
and@slack/interactive-messages
packages can be shared (or abstracted to a common dependency that can independently be used).Proposal
There's two parts of this proposal:
Move code from the
@slack/interactive-message
and@slack/events-api
GitHub repos into this repo. This does not mean those packages would be published in the@slack/client
package (see below), but rather that we'd have a monorepo that hosts code for multiple packages. This allows the documentation, test, and build tooling to be consolidated, but will require additional tooling to support publishing multiple packages from the same code repository. Maintainers will need to adjust the publishing workflow.Decompose the
@slack/client
package into three independent packages:@slack/web-api
,@slack/rtm-api
, and@slack/webhook
. Continue to publish@slack/client
as an umbrella package that simply depends on those three, and exports the same values as it does today. Adjust the documentation to point to the specific package names, so that new users can load only the code their application needs (reduce bundle size, cold-start time). It also allows the community to inspect the relative popularity of the individual parts of the Slack Platform based on npm download counts.The end state after this change is we publish 6 npm packages from this repo:
@slack/web-api
,@slack/rtm-api
,@slack/webhook
,@slack/events-api
,@slack/interactive-messages
, and the umbrella package@slack/client
. The other GitHub repos would become archived, and we'd open new issues to mirror all the open ones (or consolidate them into existing ones if they are the same). Our triage/label system would evolve to includepackage:X
as metadata to determine which packages an issue or PR affects.These changes are large enough that I think we should release them along with the next major version of
@slack/client
, even though we don't see this to be the cause for any breaking API changes.Next Steps
Investigate whether lerna would be useful to accomplish part 1 above. What are the advantages and disadvantages?
With the majority of the code being similar in
@slack/events-api
and@slack/interactive-messages
, does it make sense to turn them into one package (e.g.@slack/server
)?What do you think?
We're looking for feedback on this proposal, and would appreciate your comments. Is this change exciting and beneficial to you? Can you anticipate any problems we haven't already listed?
Happy 2019 everyone 🎉
Requirements (place an
x
in each of the[ ]
)The text was updated successfully, but these errors were encountered: