Silverstripe-framework: RFC: Use "app" folder for project code (rather than "mysite")

Created on 20 Mar 2018 · 67Comments · Source: silverstripe/silverstripe-framework

Overview

The "mysite" wording is a fairly arbitrary artefact of SilverStripe (similar to "sapphire"), and is bound to confuse users coming from other PHP frameworks like Laravel. It's less of an issue now that all composer dependencies are tucked away in vendor/, and more readily apparent that it's likely a place for custom code.

We've just started recommending this rename in the first lesson. Here's @unclecheese's rationale:

This is another entirely optional enhancement that will vary with developer preference. The default project, "mysite" is a bit tongue-in-cheek, and has overtones of a sandbox project that will never make it to production. Further, as it begins with the letter "m", it gets buried in an awkward place in the directory list. It's fairly common for developers to change the project name to something like "app", which is consistent with other frameworks, and has the benefit of appearing at the top of your directory list, which will save you precious seconds in your code editor.

Acceptance Criteria

Use app folder for new 4.x installations
Keep config in app/_config/
Explain how to add PSR4 autoloading in app/src and app/tests for custom namespaces in documentation early on
Adjust documentation examples and lessons to this standard (and point out the legacy mysite naming)
Recommend switching to app naming in 4.x upgrading guides

Folder structure:

app/
  src/
  tests/
  templates/
  _config/
public/
  index.php
  images/
  styles/
vendor/
  silverstripe/
    framework/
themes/ <- optional

Notes

Excludes any changes to support for "toplevel" modules (src instead of app/src)
Excludes default namespacing or PSR4 loading of default code in app (e.g. Page)
Upgrader ticket for auto-renames in https://github.com/silverstripe/silverstripe-upgrader/issues/60
@tractorcow reckons that we make the app naming mandatory by default in 5.x since it simplifies the recipe logic, TBC

Comparision

Drupal

Drupal has sites/default

Wordpress

I don't really understand Wordpress, but presumably this goes into wp-content/?

Laravel

Have an app folder: https://laravel.com/docs/5.6/structure#the-root-app-directory

Symfony

Has a src folder: http://symfony.com/doc/current/quick_tour/the_big_picture.html

/cc @silverstripe/core-team

PRs:

affectv4 affectv5 changminor efforeasy impacmedium rfdraft typenhancement

Source

chillu

👍8

Most helpful comment

👍 / 👎 vote on PSR-4 load app/src/, config in app/_config/

Note: if you think config should be in app/config/ see #7954

sminnee on 21 Mar 2018

👍17 👎4

All 67 comments

👍 for app already been using it for as long as I can remember, shorter, more professional and sits nearer the top of file trees.

wilr on 20 Mar 2018

I agree, but my view is that this should be an "installer" ticket not a "framework" ticket because nothing in framework should be hardcoded to either mysite or app. To the extent that we currently have some mysite hard-coding, I think we should refactor those out.

sminnee on 20 Mar 2018

👍2

Oh, the other option that I would consider is to use "" as the project module:

src/
tests/
templates/
_config/
public/
  index.php
  images/
  styles/
vendor/
  silverstripe/
    framework/
themes/ <- optional

This would model Symfony more closely, whereas app would model Laravel.

Alternatively we could pull all mysite/ folders one level up. I'm not sure if that's supported by the various manifests though.

UPDATE: OK so you mentioned this option already :P. I'm pretty sure this works as it was a precondition for the simpler-test-run work.

sminnee on 20 Mar 2018

I think app is a good option - it's used in a lot of different frameworks across languages to represent "the code for your application", and I agree with you re: src implying some sort of relationship with PSR-4

robbieaverill on 20 Mar 2018

PS: I don't think we should use Wordpress and Drupal when looking for comparisons. Both of these projects suffer from people becoming "Wordpress developers" and "Drupal developers" instead of "PHP developers", as their idioms are idiosyncratic.

When looking for comparables to model ourselves on, I think we'd do better to first look at frameworks: Laravel, Symfony, etc.

sminnee on 20 Mar 2018

👍5 🎉1

The other thing about mysite, that I didn't mention in the lesson, but wanted to, was that it gives the false connotation that SilverStripe is "a CMS for building websites."

app/ sends a much stronger message that we're a framework, and you can build, well, an app.

unclecheese on 20 Mar 2018

👍5

can we just mark this as accepted already ;)

dhensby on 21 Mar 2018

Just... yes

kinglozzer on 21 Mar 2018

Ok, but seriously, I think the app folder should actually act more as src folder, being for PSR autoloaded code and we have root level tests and config folders too... which is much more a mirror of laravel's approach:

app/
  Model/
  Control/
  ...
config/
public/
  index.php
  images/
  styles/
  resources/
vendor/
  silverstripe/
    framework/
tests/ <- optional
templates/ <- optional
themes/ <- optional

dhensby on 21 Mar 2018

👍2

OK so we have 3 broad options being recommended:

PSR-4 load app/src/, config in app/_config/ (Ingo's original recommendation)
PSR-4 load src/, config in _config/ (my preference, and mentioned in Ingo's original post as an option)
PSR-4 load app/, config in _config/ (Dan's recent recommendation)

And we need to decide which of these 3 we want to promote.

sminnee on 21 Mar 2018

👍 / 👎 vote on PSR-4 load app/src/, config in app/_config/

Note: if you think config should be in app/config/ see #7954

sminnee on 21 Mar 2018

👍17 👎4

👍 / 👎 vote on PSR-4 load src/, config in _config/

Note: if you think config should be in config/ see #7954

sminnee on 21 Mar 2018

👎10 👍5

👍 / 👎 vote on PSR-4 load app/, config in _config/

Note: if you think config should be in config/ see #7954

sminnee on 21 Mar 2018

👎11 👍8

I think that if the folder in question is going to be the auto-loaded folder, then the motivation for calling it "app" rather than "src" goes away. If I were a stranger looking at a new repo, "src" to me would say "probably auto-loaded" and "app" to me would say "probably handled in a framework-specific way".

Dan's redefinition of the app folder to just be the PSR-4 autoloaded one is good – I don't see a need for an extra folder in the path tree – but I just go one step further and say "well then let's call it src".

sminnee on 21 Mar 2018

👍2

On the fence, it's a matter of preference, having src and _config over app/src and app/_config means modules will have a similar (if not the same) structure as a regular project, which is nice.

flamerohr on 21 Mar 2018

👍2

I voted for "PSR-4 load src/, config in _config/", under the assumption that we don't need any multi-day core refactors to allow for template and config manifest priorities/sorting on an empty folder/string. @tractorcow Any insights on this?

chillu on 22 Mar 2018

Another thing to consider here: Devs often structure their codebase into "pre-modules" with the intention to split them out properly at some point. At the moment that just means creating folders on the top level, and not muck around with separate Git repos pulled in through composer. When changing to "PSR-4 load src/, config in _config/", it's technically still possible, but will look weird.

Toplevel folders for app and inlined modules

app/
  _config/
    config.yml
  src/
    Model/
maybe-module/
  _config/
    config.yml
  src/
    Model/

Toplevel folders for inlined modules only

_config/
  config.yml
src/
  Model/
maybe-module/
  _config/
    config.yml
  src/
    Model/

Modules inlined in app

app/
  _config/
    config.yml
    maybemodule.yml
  src/
    Model/
    MaybeModule/
      Model/

chillu on 22 Mar 2018

Is "pre-modules work but look weird" a bug or a #wontfix? I mean, pre-modules are weird :-P

sminnee on 22 Mar 2018

Yeah, I'm leaning towards #wontfix as well - if you namespace your config/template/php correctly, it should be easy to pull apart a subset of codebase in src/ into a standalone module. And because of namespaces, the use case of "structure your project into multiple maybe-modules for architectural cleanliness" is less common.

chillu on 22 Mar 2018

👍1

Personally I'd use app/ because it has the previously mentioned advantages of being at the top of the directory structure. The folder name src/ is just an arbitrary name (just as any other is) and we should have some justification which is better than "that's how libraries do it".

What are the reasons for each, so far I've got:

Pros for app/:

Top of dir strucutre

Pros for src/:

Other people do it

Laravel makes no attempt to explain why they use an app/ folder instead of a src/ folder, but it is, nevertheless, used as a PSR-4 autoloaded directory for the App namespace out of the box (https://laravel.com/docs/5.6/structure#the-app-directory).

A decision that we do need to make regardless of whether we use app/ or src/ is what will we declare in our default composer.json as the namespace that is autoloaded from this dir?

dhensby on 22 Mar 2018

👍2

Also, for clarity, I was actually recommending config/ instead of _config/ - I don't think the underscore provides value any longer.

dhensby on 22 Mar 2018

👍1

I voted for PSR-4 load src/, config in _config/

Thinking out loud:

the habit of using mysite/code and mysite/_config is difficult to break
if we are going to move them to root level, which makes sense to me, then I'd no longer consider mysite the app - I'd consider the whole project the app. That makes me prefer src for the "PHP" directory
if we are going with src, then config makes sense at the same level - we are after all configuring the whole app, not just the PHP code
we don't really need the leading underscore, do we?
would be nice if it was at the top I suppose - but with all the modules out of the webroot this is much less of a problem than it used to be.

It does remove the "mysite as a module" comparison, but that was relatively arbitrary anyway.

andrewandante on 22 Mar 2018

👍1

A decision that we do need to make regardless of whether we use app/ or src/ is what will we declare in our default composer.json as the namespace that is autoloaded from this dir?

Maybe App\? People can choose a new one but I can't think of a more appropriate default?

sminnee on 22 Mar 2018

Also, for clarity, I was actually recommending config/ instead of _config/ - I don't think the underscore provides value any longer.

That seems fine but I'd break that out as a separate ticket as its implementation will be separate.

sminnee on 22 Mar 2018

I'm not sure "other libraries do it" is a fair appraisal of the src/ argument. It's more that every core module does it, and most third party modules are doing it now. It's about inward-facing consistency, not outward.

unclecheese on 22 Mar 2018

👍1

However, I also value outward facing consistency. A big part of what I've tried to do with my recommendations for changes to SS4 is make it feel more like part of the PHP (or at least the Composer) ecosystem, rather than its own special ecosystem, or make it part of another framework's ecosystem.

So when I think what should a SilverStripe repo look like, I go to something like these and see how much of that applies.

https://github.com/Seldaek/monolog
https://github.com/thephpleague/flysystem
https://github.com/doctrine/instantiator
https://github.com/guzzle/guzzle
https://github.com/sebastianbergmann/php-timer

(grabbed some popular packagist repos)

sminnee on 22 Mar 2018

True, I think what I meant was:

It's about inward-facing consistency, not just outward.

unclecheese on 22 Mar 2018

@unclecheese CMS still uses a code/ folder.

Also, if we make a distinction between a silverstripe project/application and a silverstripe module, then a SS App the need for consistency between apps and modules is loosened a little.

@sminnee I agree with that idea; but those are all libraries and aren't projects and I think there is a distinction here. https://github.com/laravel/laravel is a skeleton project.

The best I can really think of are:

https://github.com/laravel/laravel (App\ => app/)
https://github.com/symfony/skeleton (App\ => src/)
https://github.com/bcit-ci/CodeIgniter (application/)
https://github.com/concrete5/concrete5 (Application\ => application/src/)

I'm really not too bothered about using src or app but I do lean towards app because I think the distinction between projects and modules is a good thing and because app will be at the top of the file list and I think that's nice ;)

dhensby on 22 Mar 2018

Yeah it comes down to Symfony-style vs Laravel-style I guess ;P

sminnee on 22 Mar 2018

OK well I'm happy to go with a broad (community-included vote on app vs src), which we've started above. Posted a deep-link to silverstripe-users#general

sminnee on 22 Mar 2018

👍3

if @flamerohr and @robbieaverill could only vote once, that'd be good :P

dhensby on 22 Mar 2018

👍1

My 2 cents - I think app tells anyone else looking at the repo that that's the code providing the main functionality, the addition on top of the off-the-shelf software like the cms and other modules.
It's what you develop to provide the added value.

src on the top level for me is any kind of source code, which in fact everything in the project is. Wouldn't indicate 'this is the main thing'.

So I voted for app/src/ and app/_config/. Next to it you can have things like app/solr/ etc.

michalkleiner on 22 Mar 2018

Is there a technical reason for having /app/src/, /app/_config etc? If all composer modules are in /vendor then the top level folder seems redundant. Why not just have /app, /config, /tests...

robingram on 23 Mar 2018

👍4

I'm gonna split _config vs config into a separate ticket.

sminnee on 23 Mar 2018

👍1

@robingram See @chillu comments above on dev's "pre-building" modules inside an existing project. I suppose if Config could distinguish between /config and /mypseudomodule/config then the only outstanding issue is one of developers ensuring looking at the dir hierarchy and scrathcing their heads for a moment..

phptek on 23 Mar 2018

I agree with @dhensby on this one. Also mimics non-php framework styles too. Look at rails for an example, all application specific files are in /app and then config is handled in /config.

ohararyan on 23 Mar 2018

👍2

@patricknelson you need to vote here https://github.com/silverstripe/silverstripe-framework/issues/7946#issuecomment-375085900 :P

dhensby on 23 Mar 2018

😄1

Trust me, I know. My response is more complicated since I'm not 100% on board with _any_ of the choices per se (underscore issue aside).

patricknelson on 23 Mar 2018

😄1

@robingram - Is there a technical reason for having /app/src/, /app/_config etc? If all composer modules are in /vendor then the top level folder seems redundant. Why not just have /app, /config, /tests...

App simply centralises all App specific code. But I understand your point, as now that Silverstripe is completely composed and all public resources are in public/ it would make sense to just have everything in root. I'm fine with both approaches.

drdrak3 on 23 Mar 2018

Technically, using app/src/ and app/_?config/ mean that "app/" is recognised by the manifest as a SilverStripe module.

Using src/ and _?config/ mean that that "" (the root path) is recognised by the manifest as a SilverStripe module.

Both work in SS4, so this isn't a barrier. This is why there were 2 different voting options.

sminnee on 23 Mar 2018

if @flamerohr and @robbieaverill could only vote once, that'd be good :P

Funny, I just removed one of my votes to keep it as one ;)

flamerohr on 23 Mar 2018

Is that one upvote, one downvote each?

hailwood on 23 Mar 2018

The downvotes are kind of redundant, as the idea is 1 upvote and 2 downvotes

sminnee on 23 Mar 2018

Except in the case of an upvote tie, the downvotes will then dictate the winner 😄 Like, right now the last two options have the same amount of upvotes.

EDIT: Relevant: https://www.youtube.com/watch?v=ussCHoQttyQ

patricknelson on 23 Mar 2018

👎5 👍5

Does it need to be a hardcoded name? Set a sensible default, but allow a developer to set their own value as a configuration setting.

silverstripe-elliot on 23 Mar 2018

I mulled it over. While it's been years since I've used Laravel (circa 4.x), I was fond of their configuration and file organization. These days they break the config files out into the root (circa 5.x) but in light of The SilverStripe Way™️ (i.e. modules in the root)...

Logical/semantic consistency between your site (or "app") code + configuration and the module folder structure wins, so I'm with the majority on this one (PSR-4 app/src/ + app/config/ for config files). Next up, if only we had:

Incorporation of configuration variation depending on environment (e.g. dev, test and live in SilverStripe nomenclature). e.g. You'd optionally have sub-folders under your config/ folder which would match these environment names with overrides depending on the current environment that's running.

patricknelson on 23 Mar 2018

Does it need to be a hardcoded name? Set a sensible default, but allow a developer to set their own value as a configuration setting.

No, but we need a default, for both the project skeleton and for all our docs and lessons.

sminnee on 23 Mar 2018

@patricknelson isn't that what

---
Only:
  environment: 'live'
---

is for?

hailwood on 23 Mar 2018

🎉1

Of course 😄 I think in slack we were sullying the use of boilerplate; I think my mind was on the magic-side of things. i.e. Utilization of folders to imply that extra configuration without having to define that in the YAML. You can easily accomplish the same effect by just adding in the extra few lines.

patricknelson on 23 Mar 2018

Recommend breaking out env-specific config folder discussion into a separate RFC. Seems like it could be added to the 4.x line as a minor change.

sminnee on 23 Mar 2018

To me src/ suggests full PSR-4 compatibility. Given that we run our own autoloader, that isn’t necessarily going to be the case - I’d wager that quite often it won’t be.

I’ve changed my vote to app/src/, partly because I feel like config/ & tests/ should live in app/ too rather than at the top level, and partly because I see it as a middle-ground between the top-level app/ and top-level src/ options. We can advocate PSR-4 compatibility and suggest people put their code in correctly named directories/files, but the top level app/ directory can hold PHP files wherever people want to put them if they choose to ignore that.

...unless anyone wants to discuss dropping our autoloader :trollface:

kinglozzer on 23 Mar 2018

👍1

Also, where does the un-namespaced Page.php fit into these options?

kinglozzer on 23 Mar 2018

Just carry over the old SilverStripe-ism of defining multiple classes in the same file. e.g.

<?php
namespace App;

class Page extends SiteTree {
    // TODO: All the things.
}

// Global namespace
namespace {
    class Page extends App\Page {
    }
}

patricknelson on 23 Mar 2018

😕1

Rats. Only minor issue is that's not auto-loaded. Maybe bootstrap that somewhere? Ok back to the same question 😄

patricknelson on 23 Mar 2018

Page.php will be the death of us, that's for sure.

andrewandante on 23 Mar 2018

😄2

Maybe for 5.x we can rid ourselves of the globally namespaced Page instance and move to having strictly App\Page. But in the interim for 4.x, that may just call for a manual class mapping entry in composer.json I guess?

Speaking of @kinglozzer:

To me src/ suggests full PSR-4 compatibility. Given that we run our own autoloader, that isn’t necessarily going to be the case - I’d wager that quite often it won’t be.

Can you think of any examples where you think it won't be fully PSR-4 compliant?

patricknelson on 23 Mar 2018

Maybe for 5.x we can rid ourselves of the globally namespaced Page instance and move to having strictly AppPage.

@patricknelson Write an RFC that outlines how this could work? :)

chillu on 23 Mar 2018

Whoa, so much engagement here - awesome to see! :) Current tally of votes:

15 👍 4 👎 for PSR-4 load app/src/, config in app/_config/
5 👍 9 👎 for PSR-4 load src/, config in _config/
8 👍 10 👎 for PSR-4 load app/, config in _config/

So I'd say we have a clear winner! Any blockers with the chosen option that would prevent us from moving forward, or would make this option a lot more work to implement?

I've also created an upgrader issue for offering to rename this automatically as part of the 4.x upgrade, to ensure the best practice is adopted more quickly.

A decision that we do need to make regardless of whether we use app/ or src/ is what will we declare in our default composer.json as the namespace that is autoloaded from this dir?

We could decide to introduce this practice alongside the change (for the two classes that are provided by the installer recipe in custom code: Page and Page_Controller). But that gets into the whole Page namespacing issue, which we should discuss in https://github.com/silverstripe/silverstripe-framework/issues/5844. Is there a reason we need to introduce autoloading for app? Seems to work fine without it, and we'll need our custom autoloader anyway until we get rid of all the implements pseudo-composition in core. But let's discuss this in a new ticket if anyone feels strongly enough to write one up.

chillu on 23 Mar 2018

nice, to be clear will we also look at making the change in the recipe-cms?

flamerohr on 24 Mar 2018

Write an RFC that outlines how this could work? :)

@chillu I'm allergic to any actual work (not really, just busy). But that's a good idea and, if anyone else is interested in taking on the challenge of writing it up, outlining the pros/cons/etc, I'd encourage it.

patricknelson on 24 Mar 2018

As I see it, there's very little value in spending time/effort in updating docs, recipes and the upgrader just to rename "mysite" to "app". People can do that themselves (and do). let's leave it as is if we aren't going to move away from our "SS way of doing things" and we can re-address this for 5.x where the scope for larger changes is more easily justified.

Do we have an idea of the effort involved for renaming "mysite" -> "app" and can we reconcile that with the value of doing so? And once we've done that, is it more worth our time than fixing first class bugs in that time instead?

dhensby on 26 Mar 2018

👍1

As a product owner, I think it's worth my team's time because of the long-term consistency it provides (both internally to the SilverStripe community, and externally to newcomers) :)

chillu on 26 Mar 2018

👍3

I'm going to post my working branches above (initial comment).