Skip to content

Questions about optimal dev setup for contributing #1254

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

Closed
rdhelms opened this issue Nov 12, 2020 · 9 comments
Closed

Questions about optimal dev setup for contributing #1254

rdhelms opened this issue Nov 12, 2020 · 9 comments
Labels
type:question Support or code-level question

Comments

@rdhelms
Copy link
Contributor

rdhelms commented Nov 12, 2020

I recently started working in this repo to make a small update to a PR. Initially, after reading through CONTRIBUTING.md, I installed the VSCode extensions mentioned in the recommended setup. Also, since the repo supposedly uses Flow, I installed the Flow Language Support extension recommended on Flow's website.

The Jasmine extension and use of the mongodb-runner package seemed to work fine with the integration tests. But the Jest extension immediately marked several tests as failing. The tests passed when run via the command line. And installing the Flow VSCode extension caused all sorts of chaos, which made me wonder if all the Flow types were even still valid.

I've started poking around at the configurations to see if any use of these tools could be salvaged, but with no luck, yet.

  • Does anyone still use the Jest extension in this repo, as mentioned in the recommended setup? If so, how?
  • Are the Flow types actually being used to validate the code in this repo? If so, how can we see/test this validation?

If the recommendations have changed, I'd ask that we update the recommended setup section of the contributing guidelines. Or if the recommended setup is still the same, I'd love some advice about how to get it up and running 🙂

@davimacedo
Copy link
Member

I don't use these extensions. @dplewis do you use them? Maybe we should remove from the contributing docs.

@dplewis
Copy link
Member

dplewis commented Nov 12, 2020

I don't anymore, I couldn't get the debugger to work back in the day. A member of our community helped build the Jasmine Test Explorer to be used with the SDK.

Looks like Flow Types are being handled by eslint-plugin-flowtype. What sort of chaos are you seeing with the Flow Extension?

@rdhelms
Copy link
Contributor Author

rdhelms commented Nov 13, 2020

Enabling the Flow Language Support extension gives me 195 errors across a couple dozen files. Additionally, I can change a completely random Flow type (like AttributeMap to number) and npm run lint still seems to pass without error. Am I missing a way to validate the code based on the Flow types?

image

@dplewis
Copy link
Member

dplewis commented Nov 13, 2020

It might be an issue with eslint-plugin-flowtype. Try to add "plugin:flowtype/recommended" under extends in the eslintrc.json file. The linter should fail. We could remove the plugin in favor of flow-bin if needed.

@rdhelms
Copy link
Contributor Author

rdhelms commented Nov 16, 2020

Adding plugin:flowtype/recommended does indeed cause npm run lint to fail. So does this mean that the current Flow types being used are not actually connected to the code, yet? That they're more for documentation than actual type accountability?

@dplewis
Copy link
Member

dplewis commented Nov 19, 2020

I think it was because there were originally a several hundred (maybe thousand) errors at the beginning and we have been updating them slowly over time. It would be nice to fix all the errors.

@dplewis
Copy link
Member

dplewis commented Nov 20, 2020

@rdhelms its mostly for documentation. Since this isn’t typescript. The Types are stripped out during the final build.

@dplewis dplewis added the type:question Support or code-level question label Nov 20, 2020
@rdhelms
Copy link
Contributor Author

rdhelms commented Nov 20, 2020

Interesting...it seems like a stretch then in the contributing guidelines to say that this repo "uses Flow" since it's intended to be used as a static type checker similar to TypeScript (TS types also compile away during final builds), but there actually isn't any type checking going on here.

@dplewis
Copy link
Member

dplewis commented Feb 15, 2025

Closing as flow type has been removed in #2145

@dplewis dplewis closed this as completed Feb 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:question Support or code-level question
Projects
None yet
Development

No branches or pull requests

3 participants