Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

[flow] switch from PropTypes to Props #2804

Closed
2 of 6 tasks
jasonLaster opened this issue May 3, 2017 · 25 comments
Closed
2 of 6 tasks

[flow] switch from PropTypes to Props #2804

jasonLaster opened this issue May 3, 2017 · 25 comments

Comments

@jasonLaster
Copy link
Contributor

jasonLaster commented May 3, 2017

We would like to switch from using PropTypes to flow props because it gives us a lot more coverage. I switched the Tabs component and learned a lot in the process.

We've done most of them, and just have a few to go!

todolist

  • Frames/index.js
  • CommandBar.js
  • TextSearch.js
  • ProjectSearch/index.js
  • Editor/index.js
  • App.js
@clarkbw
Copy link
Contributor

clarkbw commented May 4, 2017

Cool! Lemme try one! I'll take Footer

@AnshulMalik
Copy link
Contributor

I'll try one too.

@Andarist
Copy link
Contributor

Andarist commented May 5, 2017

wondering how does it work, is props stripped down later? or converted to PropTypes by a transform? I have no flow experience, so I could have missed something here

@jasonLaster
Copy link
Contributor Author

@Andarist it's stripped away. It's basically acknowledging that we're going to use flow and static type our code. Want to jump in and claim a file?

@Andarist
Copy link
Contributor

Andarist commented May 5, 2017

Yeah, already cloned and looking at EventListeners and reading dev guides ;)

Would be cool if they could be transformed to PropTypes if u distribute this as module (whatever if umd, cjs or smth else). You might have switched to flow, consumers of this as library might not.

@Andarist
Copy link
Contributor

Andarist commented May 5, 2017

@jasonLaster
can try SearchBar next

@jasonLaster
Copy link
Contributor Author

jasonLaster commented May 5, 2017 via email

@sharathnarayanph
Copy link
Contributor

I'd to like to take up "Expressions"

@Andarist
Copy link
Contributor

Andarist commented May 9, 2017

Im also working on HitMarker

@jasonLaster
Copy link
Contributor Author

jasonLaster commented May 19, 2017 via email

@anbarasiu
Copy link
Contributor

Is this an updated list? I think I'll start with shared/Accordion and shared/Autocomplete.

@jasonLaster
Copy link
Contributor Author

probably not, we try to keep it up to date :)

@ohana54
Copy link
Contributor

ohana54 commented Oct 1, 2017

I'll take Popover

@satiewaltz
Copy link

satiewaltz commented Oct 3, 2017

I'll take CommandBar!

@jasonLaster
Copy link
Contributor Author

@satiewaltz its yours

@satiewaltz
Copy link

satiewaltz commented Oct 5, 2017

@jasonLaster
Just thought I should make a note about this: I've been checking out the conversion to flow types issue in this project. Some components listed in that issue use the Context API for the shortcuts component. I've done some research into it - and it seems like a runtime type library is required to convert from contextTypes to flow types: https://github.com/codemix/flow-runtime. I think its something that should be noted in the issue when people make PRs that use that component.

@jasonLaster
Copy link
Contributor Author

Thanks @satiewaltz, I'd be open to using flow-runtime as well. Thanks for the update. Also, it's okay not to type context :) we use it very sparingly.

@amelzer
Copy link
Contributor

amelzer commented Oct 8, 2017

Did some checking

Primary Panes / Outline looks done
Primary Panes / Sources - cannot find the file, has ist been (re-)moved?
Secondary Panes / Breakpoints looks done and seems only be missing the checkmark in this ticket.
Secondary Panes / Why Paused looks also done
Shared / Button / Close looks also done
Shared / ManagedTree looks also done
Shared / ObjectInspector -> cannot find the file, has it been (re-)moved? (looks like it has been moved to another module)
Secondary Panes / CommandBar has only contextTypes left

Looks line Primary Panes / Sources Tree ist the only file left that has to be transformed.

@satiewaltz
Copy link

satiewaltz commented Oct 8, 2017

@amelzer Note: CommandBar's contextTypes can only be switched out if a runtime type library is used like flow-runtime as noted earlier from when I was looking at it. Fortunately, only several components have contextTypes defined in them: it's all places that inherits context from the shortcuts component.

Maybe that component can be marked done unless there's an alternative way to convert it that I've missed?

@amelzer
Copy link
Contributor

amelzer commented Oct 9, 2017

@satiewaltz Yes, I got that, that's why it is in this done-ish list :)

@joshunger
Copy link
Contributor

/claim

@claim
Copy link

claim bot commented Dec 26, 2017

Thanks for claiming the issue! 👋

Here are some links for getting setup, contributing, and developing. We're always happy to answer questions in slack! If you become busy, feel free to /unclaim it.

🦊 Debugger team!

@jasonLaster
Copy link
Contributor Author

@joshunger i'm taking your head off it. Feel free to jump on it again if your schedule clears up.

@joshunger
Copy link
Contributor

@jasonLaster sounds good my schedule should clear up in a week and a half

@joshunger
Copy link
Contributor

👏

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests