File structure #4

[ghost]

@jmaicaaan @dach020 @john20xdoe Pull request for #4, do guys think we need to restructure this when some things on the future comes up like a common folder?

[ghost]

Putting the client and server in src folder is ok for me, just considering some future conflicts. Note: One thing to have here on mind are the configurations, more likely the client and server should have different tsconfig.json because they don't have the same compiling style.

[john20xdoe]

I also think they should be separated, though mostly considering whether they will be deployed separately (covering their difference in the config). IF they are deployed together in one instance their difference in config will still require a central config in the root dir.

[jmaicaaan]

Putting it to the src folder is fine and also having different configs. Having a common config will need to be add in the root dir as @john20xdoe said. In the case of common folder which is available for client and server. I believe we will not have any problems with that knowing that everything insider the src would be compiled and added into dist folder as is. Hence, they will still get the folder reference into it. Also, when they've compiled, it will be using some file-loader to resolve paths rights?

src	dist
client	client
common	common
server	server

[ghost]

@john20xdoe That is the first plan and I also agree on that structure. Though @jmaicaaan and I have some discussion about using one src folder. @jmaicaaan about the compiled folders, I don't think the dist will have a common folder. Do you mean that common folder has a tsconfig.json also? For the common folder to be compiled? I am absolutely sure that the client and server has different compiling style.

[jmaicaaan]

I do have a suggestion for the problem regarding the common folder. My suggestion is to have a base config for the project and have those separated tsconfig.json extends the base tsconfig. (See attached)

Another one, I believe the config in the server and client contains .json files. With that, how can we load those things up? For example, in the server we require the abc.json which is in the server/config folder into the app. How do we resolve it? My suggestion is to make it simple and move those config files to the root and have a client and server up there. With that, config files are now free from unresolved paths.

${project_root}/config/client - All configs for client ${project_root}/config/server - All configs for server

Thoughts?

[john20xdoe]

^ this is actually good. Pros: centralized config. Cons: Though it will cause a (slight) issue if server and client are being deployed separately. I say "slightly" because such issues (e.g., bundling the correct config for the deployment folder) can be worked around as needed.

As another important concern, in cases of configs (especially if there are secrets such as passwords) we may need to rethink an approach. https://softwareengineering.stackexchange.com/questions/205606/strategy-for-keeping-secret-info-such-as-api-keys-out-of-source-control

[john20xdoe]

Related literature: https://github.com/lorenwest/node-config/wiki

[ghost]

@jmaicaaan @john20xdoe thanks for this. Moving configuration folder in root directory is good. Let me research more about simplifying our compilation, because we are having many tsconfiguration. I will be labelling this a high priority because this will be the foundation. I will get back to you guys for the report.

[jmaicaaan]

In the case of secret passwords or any important keys. What we can do there is using the environment variables approach and never ever store it in the app. With that, it is away from the source code and I believe considered as best practice for deployment. (eg: .env files)

The issue that may occur if the client and server will be deployed separately is an anti-pattern to our folder structure since we bundled them into one src folder. If we are going to deploy them separately, I believe we should not put them into one folder under src and have a client and server folder in the root.

@cedrickmandocdoc, let's get it done!

[ghost]

@jmaicaaan PR updated, I extend the tsconfig in root. I conclude that your suggestion is probably good. Using extendable typescript configuration is probably like what the angular did on their structure.Using this approach, it is easy to scale app with different environment. Anyways we are using webpack v4 and it is compiling and bundling has somehow change so it throws some warnings, I will create another issue for that. So overall after this PR merged. All tickets can now be developed.

[jmaicaaan]

Hi @cedrickmandocdoc , can you check my changes if that's okay? The things I've made are:

• Separated the creation of nest application and the starting of server. So in the main.ts it is all about the Nest Application only (set global prefixes, adding of cors, and other configs that could happen when we scale up) and in the www/index.ts it is about the starting of server. All about server and global response sender.

• If you can see in the www folder, there's an index.d.ts which is an interface for main.ts. Since the INestApplication doesn't return the instance of the created express, I returned an object contains the nestApplication and the express in the main.ts so the www/index.ts can use the expressInstance instead. Do we have to re-require the express up there? Would it be the same instance of the express we will pass in the creation of nestApplication?

• Server tsconfig.json only extends the base config. Removed other stuff because it is somehow not copying everything we have on the src folder most especially if it's too much level directories. Also, making use of the argument we set in the package.json - tsc --project ./src/server

• Added the UsersModule from the master and unit test.

• Moved some (supertest, chai) dependencies into devDependecies since they are not production dependencies.

• Make use of jest instead of mocha. It is the unit testing that nest.js suggested in the docs. Let's try another testing framework 😄

Let me here your thoughts! If everything is good, I shall merge this branch up into master then

[ghost]

@jmaicaaan this is good to go. Thank you. By the way the CI is failing because of some peer dependencies can we ignore this for now or it should be fixed?

[ghost]

I think it is because of some packages we've used in loaders for webpack. I can't just pinpoint it.

[jmaicaaan]

@cedrickmandocdoc, no we don't need to fix it on this branch. Let's create another ticket on that issue.. Looks like we need to let the CI know our node version for the peer deps.