Is the 'remove-demo' script maybe a little over-zealous?

[gargrave]

Okay, so I admit up-front: yes, user error. I should have been more careful. But I just found out the hard that the 'remove-demo' script removes more or less everything. I left the demo files in while I started working so that I could have a reference as to how things are wired up and such. After getting my head around it, I ran the script only to find that it deleted pretty much everything.

FWIW, this is not a "real" project I'm working on--mostly just trying to get my head around Slingshot, so it's not like it's a huge deal for me or anything. But looking at the script, this part seems like it might be a little overkill, as it seems to assume that the user has not already added any of their files to any of these directories:

const pathsToRemove = [
  './src/actions/*',
  './src/businessLogic',
  './src/components/*',
  './src/constants/*',
  './src/containers/*',
  './src/images',
  './src/reducers/*',
  './src/store/store.spec.js',
  './src/styles',
  './src/routes.js',
  './src/index.js'
];

I'm wondering if it should either be updated to list the specific files that need to be deleted, or maybe even just add a note in the pertinent FAQ section warning users about how thorough this is? I would hate to someone lose "real" work because they were stupid enough to do what I did. :)

I also wonder if index.js needs to be listed at all, since it is really already pretty generic and universal as it is?

[coryhouse]

That's a good point. I'm looking into adding a prompt to warn the user.

Open to suggestions.

[coryhouse]

Regarding "overzealous", I want to avoid the maintaining a list of exact files to delete. Doing so would likely just lead to bugs/orphaned files later in the remove-demo script.

[gargrave]

Yeah, I see what you mean, re: your second comment--even if you selectively deleted files, there would still be some other files attempting to import them and such, so that is no good.

I would just personally love to see a method of doing this that didn't require the "scorched earth" approach, because a lot of what gets deleted in the process is really useful (e.g. the basic setup stuff in index.js and routes.js).

The prompt, I think, would definitely be a good addition. It would at least let someone know to think twice before before "hitting the red button," and maybe do a safety commit if they have added any of their own code already.

[nicolas-leydet]

What about a hash of the demo directory generated during build. You get an error message if the hash do not correspond when the remove-demo script is called.

[j5v]

1. Perhaps use ".demo." somewhere in the filenames of files you want to remove?
2. Is it worth adding default 'do nothing' boilerplate to replace those files, e.g. a composed reducer that only returns the state, a single-route router pointing to a 'hello world' container-component pair.

Reasoning behind (2): we don't want to break the package with a remove-demo script; we want a minimal package. Is seems desirable to have an empty-as-possible codebase, where beginners can find what to change, and achieve what they want without having to write the boilerplate from an empty text file.

If we were to reduce this to a simple invocation, we could maintain two branches (demo/master and minimal), and have git do the work of unpacking either to the developer.

[gargrave]

@j5v

I had considered suggesting the exact same ".demo." solution you mentioned myself--seems like that could be a good way to handle it without having to keep an exact list of files.

On your second point, I also agree with you there--as I mentioned above, the current remove-demo script is just too thorough for my personal preference, and I have actually just been manually deleting files whenever I spin up a new project, rather than using it. I like having the boilerplate in index/routes/etc, and it's more trouble to recreate those than it is just to just delete everything I no longer need.

[coryhouse]

Good points, but I'm not sure about the .demo naming idea:

What files would be labeled .demo? For instance, should index.js for the root reducer logically be index.demo.js - given that it will typically exist in a real app as index.js? Same story with things like actionTypes.js and configureStore.js.

Let's table the file naming convo for the moment and focus solely on the behavior of remove-demo. I totally appreciate the feedback that remove-demo is being too aggressive. Anyone willing to submit a PR that tweaks remove-demo to place the app in a more useful state?

[HoraceShmorace]

Perhaps put the demo files in a /demo directory, and leave a README in the /src directory explaining that that's where the dev's actual app should go, along with an empty /src/index.js that also gets loaded by default. It could be a little confusing for true noobs, but you can only mitigate developer inexperience or incompetence so far. Though, on that note, maybe the current behavior is totally appropriate. Don't run scripts you haven't read.

[raythree]

I'm thinking that rather than a "demo app" slingshot should have a bare minimum template, complete with actions/reducers, components, containers, routes, and configureStore. A sensible minimal set would mean minimum editing to evolve it into the real app. Anything that all apps will need should stay, with //TODO comments. I really don't want to remove a demo app...I want to make minimum edits to change what is there to turn it into the app I need to develop, not start from scratch.

[coryhouse]

@raythree I believe the demo app serves the purpose you suggested. One can make minimum edits to make it the demo app if they prefer starting with a working app. It's just complex enough to show examples of all the working pieces.

If you disagree, I'm open to concrete suggestions or a pull request.

[raythree]

@coryhouse I guess you're right, but it just seemed like there was a bit much that had to be removed (the fuelSavings stuff). I guess I was thinking about a more generic container/page with just a div. I guess the test would be how many people use the remove app and start over, versus rename/edit what is there?

[raythree]

@coryhouse Actually, I'd love to propose a structure that I feel I'd use on all projects. Would you be willing to discuss changes to the way actions/reducers are structured? Even before I saw this I was trying to do exactly the same thing. I would like to see the "ducks" structure where the module's index.js is generated, and automatically exposes the types (optional), N action creators, and 1 reducer. I would even like to see it expose a convenient "getBoundActions(dispatch)". This would be better than actions, reducers, and constants being separated when they are intimately related.

[raythree]

Specifically, I'd like something like this:

// in redux/modules:
//    index.js  // generated from login and posts by pre-build from module files found
//    login.js (login duck)
//    posts.js (posts duck)


// PostsContainer.js

import { getBoundActions } from './redux/modules';

function mapStateToProps(state) { 
  return { posts: state.posts };
}
function mapDispatchToProps(dispatch) {
  return getBoundActions(dispatch).posts;
}

[kwelch]

I agree with this approach because of the simplicity of being able to refactor into domain based modules at later point. There is another enhancement (#146) that discusses some of the concerns of taking a repo like this and moving it to be organized by domain.

[raythree]

OK, I got this working as above, except for:

function mapDispatchToProps(dispatch) {
  return getBoundActions(dispatch).posts();
}

Note that posts is a function... this only calls bindActionsCreators once per set of actions, and returns the already bound actions on subsequent calls. the index.js looks like this

[raythree]

Editing to withdraw the proposal. I see the removal is leaving the index.ejs, which is useful and tied into the build. The best part about slingshort are the great build and develop tools... no app structure is going to be able to satisfy everyone, so it certainly is easy enough use the remove, and then clone your own structure into src.

[coryhouse]

@raythree Thanks for the suggestions. I'm open to tweaking the remove-demo, but I want to avoid large changes like moving to ducks, using domain folders, or dumping/moving spec files. I feel all of the latter ideas would be unfriendly to new people since it increases the number of concepts you need to understand.

[raythree]

Hey @coryhouse, I understand the goal, and I agree that any given application/test structure is unlikely to satisfy everyone. However, I would strongly encourage giving some more thought to using this kind of a structure for the redux-based components. I believe this particular area is one that especially new comers need to be guided regarding best practices. This structure logically groups action creators, action constants, middleware, and store creation. It should make it easier to grasp the concepts and reduce the number of top-level directories. Furthermore, it makes you think about reuse of reducers and choosing constants that allow such reuse without clashes. Redux-form is a good example, as it comes with a common component and reducer that you integrate into your app (including possibly adjusting the reducer mount point). Adding external/common reducers and middleware is centralized in one place.

I am refactoring my apps to use this structure, and wish I had started this way!

Cheers, Bill

[davebro]

@gargrave One thing I do when cloning any repository is push it to my own git repository right away. I too discovered "scorched earth" after running the remove-demo script, and found that I wanted certain things back. However, because I had previously pushed to my bitbucket, I simply did a "git checkout -- ." and whalah, the project was restored to its initial state, saving me the headache of re-cloning everything again. Just a thought.

[kwelch]

Are there any updates necessary to the remove-demo? Latest comments are about file structure which is out of scope for this.

I am willing to do some work on the remove-demo, but we need to define the goal of making the change.

[mikedevita]

I find that even the index.js and routes.js and reducers/index.js missing sucks if it can keep like an empty boilerplate where when you run npm remove-demo all you have to do is run npm run start -s and you'll get an empty page but no errors.

[raythree]

@mikedevita, I agree that it sucks to have to rewire the index, store, reducers, etc. to get a working starter. I know how all this stuff works, but sometimes I'm working on backend server-side stuff and can go weeks or months not doing UI work. So it's a pain to need to remember how to wire everything up!

Slingshot is awesome, well maintained, and carefully thought out. So I want to simply clone the latest version and start using it for each new project. Before I was cloning it and making modifications to fit my structure. I've been away for a while and found that a good deal had changed, including using Jest instead of Mocha.

@kwelch, I think the solution is to leave the remove-demo script exactly as it is. People can create their own starter apps that are structured exactly the way they want, and then drop that in after removing the demo. This is exactly what I did here:

https://github.com/raythree/slingshot-hello

So now I can grap the latest react-slingshot, with all the up-to-date enancements, and drop in my own app structure that I can turn into the start of a production app in under a minute, with minimum edits.

[mikedevita]

@raythree thats exactly what I did, set up a base starter clone and created a fork of it, but maintaining it in sync with this is kind of "tricky"...

[kwelch]

Understandable that keeping things in sync is tricky. If the remove demo left more files then there is a large surface ground that would need to be kept in sync.

I want to find some time to try the remove demo. Honestly I have not used it. It was added after I had already forked a version for our company starter-kit. I think once I get some time with it I will have more clarity around what may/may not have been a rub for those using it.

[raythree]

@kwelsch if you take a look at the src folder of the repo I listed above you will see pretty much the exact minimum number of files that you need to create after remove-demo, to get a fully functioning app (with routes, a store, and functioning sync and async actions). The only file there that already existed was src/store/configureStore.

Leaving configureStore was a good choice, because it does a lot of configuration for dev and production environments, and all you really need to do with it is add any middleware that you choose.

Everything else is pretty much application specific. While the ideal goal would be to have 1 component, 1 container, and 1 test case ready to go like the starter that I'm using, there is no scheme that will satisfy everyone. For example, I prefer a functional layout to the layout by type because I find it scales better for larger apps. I also like the tests in a separate (mirrored) directory.

So, I think as it stands "remove-demo" is perfect.