Open agazso opened 5 years ago
I don't have a strong opinion anymore in a big portion of these questions, the reason is that many of these standards come from other languages, and in javascript the use-cases for them kind of mesh together / overlap.
constant declarations: I think the UPPER_SNAKE_CASE
is appropriate if it's used in several places, the usage is not tied to one file. If more then 2 files use them, ideally they can go to a separate file. Related topic: readonly
is a TypeScript property modifier. It even disallows usage of mutating apis on arrays. Since const
keyword is only useful for primitive types and forbidding reassignment, we could use it for more complex structures. There is also a type which makes its parameter's properties readonly. Eg.:
type DefaultAppState = Readonly<AppState>;
const defaultAppState: DefaultAppState = {....
Probably not a good example but you get the idea.
free functions: for namespacing we can use a convention at the import side (* as ...)
classes vs. free functions & interfaces: not the best place for code review, but I'll share my thoughts: all the functions are exported, and also used elsewhere. I think the implementation should be only accessible the instance generated by the factory. In that case you can also declare the variables inside the factory function, or as a default parameter if that's more suitable in some cases.
code structure opinion: I think we should introduce another equally widespread method for organization, create folders for functionality, and put everything there, in our case: tests, components, containers. I would even group related things under one folder, e.g. all the views using RefreshableFeed should go under one folder. Put general UI components in separate ui folder: buttons, navheader, etc.
constants: I agree in general, but then in your example, shouldn't defaultAppState
be called DEFAULT_APP_STATE
instead? My feeling is that for primitive type constants and for enums we can use UPPER_SNAKE_CASE
and for default objects and creator/factory functions we can stick with lowerCamelCase
(because what's the difference between a factory function without arguments and a constant anyhow if there are no side effects?). Then UpperCamelCase
can be reserved for types.
free functions: ok, that's what I started using recently
classes vs. free functions & interfaces: I don't understand the 'all the functions are exported` part. Can you explain what do you mean here? There is also another problem I ran into recently, that I had to export some internal functions in order to be used in tests. Then they become part of the module interface and then can leak out the implementation to other parts of the code.
code structure: I am not arguing that's it's a good idea or not, but by following that logic don't we end up with lot of folders with only one single file in them? When is the right time to create a folder?
UPPER_SNAKE_CASE
for primitivescontinue discussion from https://github.com/agazso/postmodern/pull/151 about returning SomeValue | undefined
We should consider using the pattern mentioned here: https://basarat.gitbooks.io/typescript/docs/javascript/null-undefined.html or using Maybe
?
We should standardise coding conventions, the things that are not covered by TSLint.
We use different conventions in different files and it is very inconsistent sometimes. It can be very confusing when switching between different files adhering different standards. Here are some issues that I am aware of:
const naming: we are trying to use const as much as possible, which is a good thing and makes the code more understandable. However in Javascript you end up using it for variable declaration, function declaration besides actual constant declarations. For variable names (including function names) we use
lowerCamelCase
, but for actual constant definitions we are using a mix ofUpperCamelCase
orUPPER_SNAKE_CASE
.free functions vs. static members for utility functions: sometimes we put free functions in a file and directly export them (e.g.
random.ts
) and sometimes we create classes with static members and export the class (e.g.Debug.ts
). I prefer the former (the latter is mostly very early code anyhow) but I like the 'namespacing' that comes with the latter, because it allows you to have shorter (and sometimes colliding) function names.file names: some of our file names start with uppercase, some of the starts with lowercase. There seems to be an organising force behind it: files that define types starts with uppercase, files containing only function definitions starts with lowercase, but there are some exceptions.
classes vs. free functions & interfaces: most of the time we are using free functions, functional style. This style can be a bit cumbersome, for example when you want to wrap some context or configuration in several of your function calls. In these cases we use classes, but when using classes for pure functions you have to use
this
and that itself can be problematic in Javascript. I already experimented with a different style inSwarm.ts
with theFeedApi
: you implement an interface within a factory function, that can also extend the interface, store variables as closure etc.