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
[Submissions Closed] Participating in #HacktoberFest? Here's some beginner friendly ways to contribute to CodeSandbox! #2621
Comments
This is really awesome I would like to work on: |
I would like to refactor |
#2623 is an open PR for the refactor of |
Hello, I will be working to refactor: |
Hey! PR #2630 refactors |
Hi. I'd like to refactor this one |
Hey! Thank you. |
Hey! Thank you. |
Hello! Thank you. |
I will be working with |
Hi 👋🏽 |
Hello, |
Hi, I'm interested for |
Hi, |
Hey! PR #2655 refactors /app/pages/Sandbox/SignOutNoticeModal/index.js |
Updated checklist #2621 (comment) |
Starting |
Hi. I'd like to refactor this one |
PR for |
Hi, I am taking up /app/pages/common/Modals/PreferencesModal/Experiments/index.tsx |
i am taking up |
I am taking up |
Please review #2918 |
Meanwhile I am taking /app/pages/Sandbox/Editor/Workspace/OpenedTabs/index.js |
PR [#2922] for |
Updated checklist #2621 (comment) |
PR[#2926] for |
Hi |
it looks like some components are already refactored and using overmind:
|
Updated checklist #2621 (comment) @Tarektouati /app/pages/Profile/Showcase/index.js (existing PR #2766) @22PoojaGaur /app/pages/Sandbox/Editor/Workspace/OpenedTabs/index.js (existing PR #2769) @nguanh /app/pages/Sandbox/Editor/index.js added claim @jhsu /app/pages/Sandbox/Editor/Workspace/Dependencies/AddVersion/index.js (Completed #2768) |
Hi i can work on |
I'm not part of the codesandbox team. I was simply trying to help manage the checklist so people would know what was already done/being worked on. I believe the team is working on reviewing the PRs this week. They stated they will process them in PR order, so if you worked on a PR that's already been submitted, they may close yours (assuming the initial PR was fine). Anyway, good luck to you all. Happy #HacktoberFest |
Saeris commentedOct 3, 2019
•
edited by MichaelDeBoey
Update 2: Thanks everyone who participated! We're asking that no one open any additional PRs at this point for Hacktoberfest. We've still got a lot of submissions to review and merge before the 31st and we'd like to make sure that everyone who contributed gets credited for their work.
Update: We've received a ton of submissions from this and we're really grateful to everyone who's participated this year! As you may have noticed, we're a bit behind on merging your PRs and updating the list of components to be refactored. This week we're going to go through all the remaining open PRs in order of submission and either request changes or merge them. Going forward, we're sticking with the first-come first-served approach, meaning any duplicate submissions will simply be closed. If you're still considering making a contribution, you can up your chances of getting your PR merged if you can make sure someone else hasn't beaten you to that part of the code base by searching through all of the Hacktoberfest PRs first.
Hey everyone! As you might already know, this month is #Hacktoberfest, which is an event that encourages people (especially newcomers) to participate in open source development. Since CodeSandbox's client is an open source project, we thought it would be a good idea to throw together a small guide on how you can make some quick contributions and rack up those PRs.
Right now we're working on overhauling the code base to transition everything to Typescript and replace our old state management solution (Cerebral) with Overmind (both developed by our own @christianalfoni). We're at a point where a lot of the groundwork for these big refactors has been done, and now all that's left is to implement changes across the application. We've come up with some patterns and best practices around how to do this, which we'll explain below.
About the Event
But first let's quickly go over Hacktoberfest and how you can use this issue as a guide to participating in the event. Here are the rules as explained on the official website:
To help you get started, we've put together a list of components that still need to be refactored to use Overmind. We'd ask that you please choose up to 4 components from this list and create separate PRs for each. You can reply to this issue to let other participants know which components you plan on refactoring (and using your comment, we'll mark who's doing what on the list to help people choose). We'd like to make it so as many people as would like to be able to participate in the event, so that's why we would appreciate a maximum of 4 contributions per person.
Contributing
First, if you've never contributed to CodeSandbox before, we'd recommend you check out our Contributing Guide which covers some basics to help you get started running the project locally and how to submit your code.
To make a Hacktoberfest contribution, please create a new PR with:
For the refactor itself, in some cases it's really rather simple. Many of our components have already been re-written as functional components which use React hooks (in some cases, a component may need to be additionally refactored from a class component to a functional component). As part of our transition from Cerebral, we used to some glue code in our initial tests, which follow a pattern similar to this:
To this:
Please take note how we were previously using the
inject
andhooksObserver
higher order components to passstore
andsignals
as props to the component. Those get replaced with a newuseOvermind
hook that returns an object containing the keysstate
andactions
which are synonymous withstore
andsignals
. In most cases these simply need to be swapped out as in the example above (which also includes some extra tidying of types and code style changes).One of the greatest benefits of this refactor is that Overmind now provides us full Typescript support for our global state management. However that does mean that in some cases there may be type errors that also need to be resolved. To go about resolving these, you may need to do one or more of a few things:
app/overmind/namespaces
)What I've found is that this guide is a great resource for learning how to write types for React applications. I first check that before hitting up Google and StackOverflow for answers to type errors I encounter. Typescript can be a little difficult, but for the most part this refactor doesn't involve any of the really advanced Typescript wizardry.
So to summarize:
inject
andobserver
/hooksObserver
are to be removed, also remove the import at the top of the fileuseOvermind
hook fromapp/overmind
import:yarn typecheck
andyarn lint
, fix any errors that crop up (warnings can be ignored)Components to be refactored:
(@2759)] /app/pages/common/Modals/PreferencesModal/CodeFormatting/Prettier/index.js
(
[Closed by @christianalfoni] /app/pages/Sandbox/Editor/Content/index.js(
(
/app/pages/Sandbox/Editor/Workspace/Project/Project.tsx
(
Best Practices and Style Guide
We've been working on a set of coding guidelines that we're applying across the project to increase readability and consistency. While not hard rules, we'd ask that you stick to these a close as possible. Since these are mostly stylistic choices, they're unlikely to cause anything to break if not followed, but we may ask you to fix a few if we feel it's important.
this
)index.ts
file handling all exports to be consumed by outside componentsinterface
overtype
for typing Propsconst SomeComponent: React.FC<ISomeComponentProps> = ({ . . . }) => { }
overconst SomeComponent = ({ . . . }: ISomeComponentProps) => { }
to avoid needing to define common React component propstypes.ts
file adjacent to the component to reduce clutter in the component definitionqueries.gql
andmutations.gql
to reduce clutter in the component definitionGlobalStyles.ts
file adjacent to the root component that requires itcss
tagged template to enable style linting/syntax highlighting.In general, our component file structure looks like this:
Simple components (those without any styles, few types, and no grapql) live in single named files like
SomeOtherChildComponent.tsx
, while others live in their own directories with folder names that match the main component's name, usingindex.ts
to define all exports to be exposed to all outside consumers up the tree and files such aselements.ts
to handle concerns like component styling, shared type definitions, graphql queries/mutations, etc.Many of our existing components just live in their own directory and are defined in the
index.js
file there. If you encounter one of these, it's best to split it up according the the rules defined in the style guide. If it's a simple component, you can just rename the file to the component's name and move it to the parent directory, getting rid of the folder it's in entirely.If you're confused by any of these, just try your best and we'll point out any changes we'd like you to make when we review your PR! Don't be afraid to as us questions too!
Other Quick Wins
So while Overmind is the main focus of our current refactor, we're also looking to convert more of the code base over to Typescript and React Hooks. If you'd like to try and refactor any of our JavaScript files or convert a class component, you're more than welcome to do so! We don't have a convenient list of files that need to be refactored for those, but it shouldn't be too hard to find.
In general we could always use contributions like bug fixes and better documentation, so if you spot an issue you'd like to try and fix or something in our docs has a typo or isn't clear enough, please feel free to submit a PR for those too!
Thanks and Happy Hacking!
Lastly, we'd like to extend our sincerest thanks to our community for all the feedback and support you give us! The entire CodeSandbox team is driven by your enthusiasm and we're always on the lookout for new ways to collaborate together and build new features to address all the awesome use-cases you come up with for our app. Thanks so much for your time and effort!
The text was updated successfully, but these errors were encountered: