CVarisco / create-component-app

Tool to generate different types of React components from the terminal. 💻
MIT License
859 stars 58 forks source link

Does not work with no config or example configs #55

Closed ds17f closed 6 years ago

ds17f commented 6 years ago

If I run without a config file specified I get: "Bad config file, Please check config file syntax." Then if I say I want to use a template I get: "Object.entries is not a function"

I tried using the example config files and each fails with a "Bad config file" message. It appears that the comments in the examples are invalid. It also looks like, with the basic .ccarc config that is presented the templates don't work.

I'd love to update the readme with some corrections but I'm not sure I know what all the options should be. Can you post a working config file so that we can update the docs to better guide a new user?

Prioe commented 6 years ago

Hey @damiansilbergleithcunniff

Your first issue ("Bad config file, Please check config file syntax.", without a config file present). Was caused by us throwing an error if there is no config file present. I believe this shouldn't happen and I implemented a more friendly info message if we can't find a configuration file.

My pull request also includes a possible improvement to the docs, I removed the examples, created a table which documents the various configuration options and created an example config, which is referenced in the readme.

The error "Object.entries is not a function" you receive is caused by your node version not supporting Object.entries. We do transpile the code with babel to version 6, but I also experienced this error at node v6.11. I believe this happens because the don't include babel-polyfill in our build. We can either increase the required node version or include the polyfill.

What do you think @CVarisco

CVarisco commented 6 years ago

First of all guys thanks a lot for your support, I really appreciate 🎉

Bad config file, Please check config file syntax.

You are right @Prioe and with your PR should be fixed 🙂

The error "Object.entries is not a function"

Exactly, but I prefer to don't include the polyfill. We need to help people update their own version of Node because in this way developers can have all the new features/fix/performance from the new version. So, I think that is better to increase the node version to 7.

What do you think @Prioe and @damiansilbergleithcunniff ?

ds17f commented 6 years ago

@Prioe you're awesome. Thanks for jumping in here. I reviewed your changes and they make sense to me.

@CVarisco I completely understand your motivation for leaving out the polyfill. This actually explains why your app worked on one of my machines (running node 7.7.3) and not on another (running 6.x.x). I chose to use 6.x on the second machine because I'm working on a project that will run in AWS Lambda. Currently, Amazon supports version 6.10 as the latest nodejs. I understand that I can use nvm and run multiple versions of node, but I'd rather keep things consistent so that I know the code that I'm building will run in lambda.

Forgive my lack of knowledge as I'm still learning. Would it be possible to provide instructions on how to add the polyfill so that somebody running an older version of node can still use your app? Regardless, the documentation should be updated to clarify this issue for future users.

@CVarisco, excellent work on this project. I look forward to getting it running properly and using it with the devs on my team to create a common standard for the way we build components in our projects. Well done. Thank you.

CVarisco commented 6 years ago

So, I merged the PR of @Prioe and I added the polyfill to make that every developer can use create-component-app 🙂

Thanks again for your support guys 🎉 🎉 🎉 🎉

PS: @damiansilbergleithcunniff Could you test with the 1.4.2 version and tell me if it works well (I tested with Node v6 and works)?

ds17f commented 6 years ago

@CVarisco I updated using npm install -g create-component-app which gave me 1.4.2. I then ran the app and tested. The new messaging is MUCH more clear and the polyfill appears to have solved the other issue as well. This gets a big 👍🏻 from me.

Thanks again @Prioe for your help. You guys rock!