Skip to content
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

ref(electron): define project source code directory structure #126

Closed
aesedepece opened this issue Jun 11, 2018 · 7 comments
Closed

ref(electron): define project source code directory structure #126

aesedepece opened this issue Jun 11, 2018 · 7 comments
Labels
enhancement 📈 New feature or request help wanted 🙏 Extra attention is needed question ❓ Further information is requested

Comments

@aesedepece
Copy link
Member

As we start to devote more efforts to the front end side of Sheikah, our project source code will naturally tend to break separation of concerns unless a directory structure policy is defined and enforced ASAP.

Being one of the most popular Electron powered projects, behold Atom for reference:

Electron
  ├── atom/ - C++ source code.
  |   ├── app/ - System entry code.
  |   ├── browser/ - The frontend including the main window, UI, and all of the
  |   |   main process things. This talks to the renderer to manage web pages.
  |   |   ├── ui/ - Implementation of UI stuff for different platforms.
  |   |   |   ├── cocoa/ - Cocoa specific source code.
  |   |   |   ├── win/ - Windows GUI specific source code.
  |   |   |   └── x/ - X11 specific source code.
  |   |   ├── api/ - The implementation of the main process APIs.
  |   |   ├── net/ - Network related code.
  |   |   ├── mac/ - Mac specific Objective-C source code.
  |   |   └── resources/ - Icons, platform-dependent files, etc.
  |   ├── renderer/ - Code that runs in renderer process.
  |   |   └── api/ - The implementation of renderer process APIs.
  |   └── common/ - Code that used by both the main and renderer processes,
  |       including some utility functions and code to integrate node's message
  |       loop into Chromium's message loop.
  |       └── api/ - The implementation of common APIs, and foundations of
  |           Electron's built-in modules.
  ├── brightray/ - Thin shim over libcc that makes it easier to use.
  ├── chromium_src/ - Source code copied from Chromium. See below.
  ├── default_app/ - The default page to show when Electron is started without
  |   providing an app.
  ├── docs/ - Documentations.
  ├── lib/ - JavaScript source code.
  |   ├── browser/ - Javascript main process initialization code.
  |   |   └── api/ - Javascript API implementation.
  |   ├── common/ - JavaScript used by both the main and renderer processes
  |   |   └── api/ - Javascript API implementation.
  |   └── renderer/ - Javascript renderer process initialization code.
  |       └── api/ - Javascript API implementation.
  ├── spec/ - Automatic tests.
  ├── electron.gyp - Building rules of Electron.
  └── common.gypi - Compiler specific settings and building rules for other
      components like `node` and `breakpad`.

Ok, that's a overly complicated when compared to our modest structure, but I think it brings some good ideas.

As you can see, the lib directory is divided into browser (code run in the main process), renderer (code run by the renderer process) and common (shared code). Both renderer and common target "browser JS", while code browser is NodeJS-specific.

This could be our structure for the time being:

Sheikah
  ├── app
  |   ├── actions
  |   ├── components
  |   ├── containers
  |   ├── lib
  |   |   ├── common
  |   |   |   └── api
  |   |   ├── main
  |   |   |   ├── api
  |   |   |   ├── crypto
  |   |   |   ├── encoding
  |   |   |   ├── storage
  |   |   |   └── utils
  |   |   ├── renderer
  |   |   |   └── api
  |   ├── reducers
  |   └── store
  ├── config
  ├── docs
  ├── resources
  ├── tests
  └── typings

We should give a look to some other projects with similar stacks. Counterproposals are expected 😛

@aesedepece aesedepece added enhancement 📈 New feature or request question ❓ Further information is requested labels Jun 11, 2018
@aesedepece aesedepece added this to the Sheikah Sprint #3 milestone Jun 11, 2018
@aesedepece aesedepece added the help wanted 🙏 Extra attention is needed label Jun 13, 2018
@aesedepece
Copy link
Member Author

@witnet/devs comments?

@anler
Copy link
Contributor

anler commented Jun 14, 2018

This is my proposition:

sheikah
  ├── app
  │   ├── common              Code shared across main and renderer
  │   ├── main                Main process "the backend"
  │   │   ├── api
  │   │   ├── crypto          Do not use lib/ here, just move
  │   │   ├── encoding        its content under main/. These act also
  │   │   ├── storage         as adapters between our logic and lib/
  │   │   └── ...
  │   └── renderer            Rendering process "the fronted"
  │       ├── api
  │       ├── crypto          Do not use a lib/ here, just move
  │       ├── encoding        its content under main/. These act also
  │       ├── net             as adapters between our logic and lib/
  │       ├── ...
  │       └── ui              The UI itself, redux, react, etc
  │           ├── actions
  │           ├── components
  │           ├── containers
  │           ├── reducers
  │           └── store
  ├── config
  ├── docs
  ├── lib                      Custom libraries that has no dependencies
  │   ├── crypto               to our bussiness logic. Code here has the
  │   ├── encoding             potential to be moved into their own
  │   ├── storage              npm package and distributed separately and
  │   └── ...                  ideally work in both browser and node
  ├── resources
  │   ├── common               Common resources
  │   ├── main                 Resources for the main process
  │   └── renderer             Resources for the renderer process
  ├── tests
  └── typings

@aesedepece
Copy link
Member Author

@anler I like it.

Only objections are store, reducers and actions. These are Redux folders that will contain code defining the app state data model, mutation logic and API, respectively.

I'd take store out of ui and put actions and reducers inside store itself.

About the requirement for the code in lib to be enviroment-agnostic, I agree with you. Of course that means more WebPack configuration in order to polyfill Buffer and many other NodeJS sugary features.

@anler
Copy link
Contributor

anler commented Jun 14, 2018

Only objections are store, reducers and actions. These are Redux folders that will contain code defining the app state data model, mutation logic and API, respectively.

I'd take store out of ui and put actions and reducers inside store itself.

@aesedepece that sounds perfect

About the requirement for the code in lib to be enviroment-agnostic, I agree with you. Of course that means more WebPack configuration in order to polyfill Buffer and many other NodeJS sugary features.

We can always start by writing those inside app/main or app/renderer (tied to a specific environment) and move them into lib/ later when we need them to be environment-agnostic.

@aesedepece
Copy link
Member Author

aesedepece commented Jun 14, 2018

#144 is the implementation follow-up of this issue.

anler pushed a commit to anler/sheikah that referenced this issue Jun 14, 2018
BREAKING CHANGE: everyone need to update what they are working on in order to follow the new project
structure

fix witnet#144
anler pushed a commit to anler/sheikah that referenced this issue Jun 14, 2018
BREAKING CHANGE: everyone need to update what they are working on in order to follow the new project
structure

fix witnet#144
anler pushed a commit to anler/sheikah that referenced this issue Jun 14, 2018
BREAKING CHANGE: everyone need to update what they are working on in order to follow the new project
structure

fix witnet#144
anler added a commit that referenced this issue Jun 15, 2018
* refactor(file name): implement project layout defined in #126

BREAKING CHANGE: everyone need to update what they are working on in order to follow the new project
structure

fix #144

* chore(tslint): turn off align rule due to conflict

the tslint rule for aligning blocks of code and typescript-formatter
are in conflict because the formatter doesn't like aligning parameters
of a function, see microsoft/TypeScript#11629
@mmartinbar
Copy link

mmartinbar commented Jun 15, 2018

I'm integrating the work on #27 into the new project structure and I wanted to get your opinion on this proposal for the IPC methods and handlers:

app
├── common
│   ├── messages.ts
│   └── methods.ts
├── main
│   ├── api
│   │   ├── handlers
│   │   │   ├── index.ts
│   │   │   ├── nop.ts
│   │   │   └── ping.ts
│   │   └── lib
│   │       ├── electronIpcMain.ts
│   │       ├── genericIpcMain.ts
│   │       └── ipcBackend.ts
│   ├── chain
│   ├── ...
├── main.electron.ts
└── renderer
    ├── actions
    ├── api
    │   └── lib
    │       ├── electronIpcRenderer.ts
    │       ├── genericIpcRenderer.ts
    │       ├── ipcFrontend.ts
    │       └── mockedIpcRenderer.ts
    ├── components
    ├── containers
    ├── ...
    
test
├── app
│   ├── common
│   │   ├── messagesFixtures.ts
│   │   ├── messages.spec.ts
│   │   ├── methodsFixtures.ts
│   │   └── methods.spec.ts
│   ├── main
│   └── renderer
│       ├── api
│       │   └── lib
│       │       └── ipcFrontend.spec.ts
│       ├── components
│       ├── containers
│       └── reducers
├── e2e
├── ...

What do you think, @aesedepece, @anler, @witnet/devs?

@anler
Copy link
Contributor

anler commented Jun 15, 2018

@mmartinbar looks 👌 but here are a couple of suggestions based in my personal taste:

  • I would rename app/main/api/lib to app/main/api/ipc and remove IpcMain from the names inside it
  • same thing for app/renderer/api/lib

😬

mmartinbar pushed a commit to mmartinbar/sheikah that referenced this issue Jun 15, 2018
Adapted IPC work to project layout defined in witnet#126

witnet#27
anler pushed a commit to anler/sheikah that referenced this issue Jun 21, 2018
Adapted IPC work to project layout defined in witnet#126

witnet#27
anler pushed a commit to anler/sheikah that referenced this issue Jun 21, 2018
Adapted IPC work to project layout defined in witnet#126

witnet#27
anler pushed a commit to anler/sheikah that referenced this issue Jun 21, 2018
Adapted IPC work to project layout defined in witnet#126

witnet#27
anler pushed a commit to anler/sheikah that referenced this issue Jun 21, 2018
Adapted IPC work to project layout defined in witnet#126

witnet#27
anler pushed a commit to anler/sheikah that referenced this issue Jun 25, 2018
Adapted IPC work to project layout defined in witnet#126

witnet#27
anler pushed a commit to anler/sheikah that referenced this issue Jun 25, 2018
Adapted IPC work to project layout defined in witnet#126

witnet#27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 📈 New feature or request help wanted 🙏 Extra attention is needed question ❓ Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants