-
-
Notifications
You must be signed in to change notification settings - Fork 115
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
Move codebase to ES6 #367
Comments
We could also go one step further and move to TypeScript. 😄 |
We could but I have no TS experience at all so to me it would take much more time to get into the ts thinking |
Alright let's take it step by step. |
I think a good starting point for moving towards TS would be to add type definitions at least, so intellisense can pick it up better. |
I'm willing to do this. |
Great @rashmod !!! Please do one PR per package (Thie repo contains multiple packages). Since you are new it may be a good start by using a smaller package. Another thing that's still not discussed is which linter we use cc @StorytellerCZ @denihs |
Okay, then I'll start with the smaller packages. But when you say convert to es6 are you just talking about changing the |
I woul say use Also favour destructuring and named parameters incl. parameter defaults where applicable. Note: arrow functions should be used with care since there are many binding going in Blaze with functions using Don't worry, just start and we will review anyway. If there are things left to be improved etc. we will let you know and you will have enough support to get this done right 👍 |
I will start working on this now. But @jankapunkt you were talking about the linter to use.....? |
I just checked and the main Meteor project uses the @quave/quave config (see their package.json) which is maintained by @filipenevola so I think we should also adapt to use it. What do you think @StorytellerCZ @denihs |
No objection from me |
I'm having some issues while setting up the local env. I used nvm to change to node v14.20.0 and somehow installed meteor (faced many problems with this too). But when following the steps in the readme sidenote: my primary cli git bash does not work with meteor. "bash: meteor: command not found" even tho it works using PowerShell or cmd |
I am unfortunately only on Linux and MacOS so I cannot be of much help regarding windows setup. Have you checked the Meteor forums about these issues? IIRC there should be a few solved topics on windows issues regarding installation of Meteor. Can anyone else help out here? |
The error I keep getting is that there is no package.json file. And according to my beginner-level understanding, you need a package.json to do npm install and stuff |
You need to move into the test-app directory. It contains the package.json file. The contribution.md file covers the commands you can use there. This is to separate the test project from the packages. |
This would be a big task but I think there will be lots of benefits in terms of code-readability (especially with template-literals) and a few performance improvements either.
This could be also distributed to PR-per-package:
In an optimum there would be no changes to the functionality at all.
The text was updated successfully, but these errors were encountered: