Re-factor admin UI into its own package and allow custom field types

[geloescht]

I started on a fork in which I am moving the Admin UI into it's own module. It is a work-in-progress, but already capable of running the server with and without the admin UI.
I propose to do this sooner, rather than later when a working plugin / package architecture is in place (in reference to #912 ). This way we can cut unnecessary ties one by one, until keystone is capable of auto-detecting the presence of the admin package. Until then, imho it's no worse to explicitly refer to a package named keystone-admin than it is to have it non-packaged within a sub-directory. What do you think?
Maybe the admin API should also be split off into it's own package, so it can be re-used globally.

Notes:

• We can even have the module (and more modules) within the repo without any problems until it can be moved to its own.
• I refactored some relative paths to begin with the package name, like require('keystone/bar') instead of require('../../../bar'). I believe this increases modularity and facilitates moving files around. However, it requires that you develop keystone within a directory named node_modules.
• NPM's peerDependencies are used to make the packages depend on each other. optionalDependencies are used to install the admin UI together with the main package. bundledDependencies could be used as long as the admin UI is kept within the same repo.

[geloescht]

Note to myself: #2088

[geloescht]

Since the latest commit it is possible for application developers to add their own field types and custom React components for the admin UI.
To include components from the admin UI in React components for field types, use import ... from 'keystone-component/...'. The list store can be included using import Lists from 'keystone-stores/Lists'. remapify is used for this and is a new dependency.
Bundles are rebuilt automatically, as new components are registered. This might be a little inefficient.

@JedWatson: Please review my pull request. I am aware, that this is a rather large and controversial restructuring. So if you feel, that certain parts are not acceptable or need more work before being accepted, please give me some pointers what should be changed or backed out.

[webteckie]

I like this! @JedWatson what do you think? Does it completely disrupt the 0.4 plans? :-) @geloescht, one comment I have is that I think it would be more desirable to put the keystone-admin project directly under keystonejs and add it as a dependency to keystone.

[geloescht]

@webteckie I also think keystone-admin should live outside keystone, just as keystone-utils does. However, I see no problem of keeping it as a bundled dependency until it is ready to be exposed and we are confident that no more files need to be moved between the modules.

[webteckie]

@geloescht if this breaks on windows it's a no-go for me. It must work on windows as keystone has been working just fine on windows.

[JedWatson]

@geloescht I've been reviewing, it's large :)

This is definitely going in the right direction and the prototyping is valuable. It's 100% something I was planning to do after 0.4 is launched, targeting 0.5 (or probably just jumping to 1.0) as the next version.

keystone-admin would be published as a separate repo and separate npm package which would resolve the issues with the way this has been refactored in-place in the node_modules directory (which is not real-world robust but a viable workaround for prototyping)

My biggest concern doing this now is it may reduce the speed of finalisation of 0.4 which I'm really hoping to get out this month. The big things are all done and we're writing unit tests right now, but while there's still potentially a lot of code being moved between keystone core, lists, fields and the Admin UI putting it in a separate repo and package may increase churn quite a bit.

Worth noting that longer term I'm also hoping to spin most of the field types out into their own packages, and the List stuff into a keystone-schema (or similar) package eventually. Need to work out how that integrates with stuff like keystone-graphql which is in development though; the wrong separation here could corner us in the future so I'm being conservative while we focus on simply adding tests & finalising architecture.

[JedWatson]

@webteckie does this work on Windows?

[geloescht]

@webteckie The breaking on Windows is only temporary, until #2383 has a proper solution that works on Travis CI. The solution which is currently implemented here uses a symlink and git might therefore break it on Windows.

@JedWatson I see. I like the idea of 0.4 being released soon, so I got no complains if this gets postponed until development on 0.5 starts. As long as there is no other big refactoring happening until then it should be comparatively easy to keep this PR up to date.

[kadosh1000]

I think this is my main issue with Keystone, custumization of the admin and the field types is the most important missing feature in keystone. Hope to see these features asap

[JedWatson]

@kadosh1000 those are our two primary focuses to unblock with the next version. FWIW we have some projects going at Thinkmill that need these capabilities, which are part of funding the bulk of Keystone's development, so you can bet we're going to see them soon, and we're making rapid progress towards the next release.

[geloescht]

After a long and painful rebase, this PR is updated to the latest master. I skipped the changes that added the keystone-components and keystone-stores remapping. Many of those components have moved to fields/components anyway or can just as easily be pulled directly from the keystone-admin package.

Another thing that came to my mind: (and was actually recently mentioned here: keystonejs/keystone#2824 (comment) )
When moving to production, bundles should probably not be dynamically rebuilt each time the server restarts. However, my mechanism depends on dynamic rebuilding, as field types can be added and removed at any time.
Should registration of field types be static instead? That would force application developers to add an extra building step before deployment, if they want custom field types. It could confuse (new) users who are not aware of this requirement and add inconvenience for everyone.
IMO, a best-of-both-worlds approach would be to cache bundles between restarts of keystone. We could restrict adding custom field types to any point in time before browserify starts building, for example keystone.start(). Otherwise we might be forced to keep multiple versions of a bundle in the cache. A pre-built fall-back with only built-in field types should be kept around for the case the application developer introduces errors.

[mxstbr]

Awesomee, thanks so much! Will have to think about your points about the bundling… /cc @JedWatson

[webteckie]

@geloescht I quickly tested this on windows and this is what I get when I start the e2e server with this PR:

$ export KEYSTONEJS_PORT=9999 && node test/e2e/server.js --notest
Note: Optional package keystone-admin is not installed. Keystone will run in
      headless mode. run `npm install keystone-admin` to install the KeystoneJS
      admin UI.
module.js:328
    throw err;
    ^

Error: Cannot find module 'underscore'
    at Function.Module._resolveFilename (module.js:326:15)
    at Function.Module._load (module.js:277:25)
    at Module.require (module.js:354:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (C:\Users\Carlos\git\prs\pr2390\keystone\lib\schemaPlugins\localize.js:1:71)
    at Module._compile (module.js:410:26)
    at Object.Module._extensions..js (module.js:417:10)
    at Module.load (module.js:344:32)
    at Function.Module._load (module.js:301:12)
    at Module.require (module.js:354:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (C:\Users\Carlos\git\prs\pr2390\keystone\lib\schemaPlugins.js:5:20)
    at Module._compile (module.js:410:26)
    at Object.Module._extensions..js (module.js:417:10)
    at Module.load (module.js:344:32)
    at Function.Module._load (module.js:301:12)

Shouldn't the admin ui app be installed by default?

[webteckie]

And so I ran the command to install the admin ui app and still go the same thing:

$ npm install keystone-admin
npm WARN optional dep failed, continuing keystone-admin@^0.4.0-alpha

Carlos@XPS8500 MINGW64 ~/git/prs/pr2390/keystone (geloescht-clean-up)
$ export KEYSTONEJS_PORT=9999 && node test/e2e/server.js --notest
Note: Optional package keystone-admin is not installed. Keystone will run in
      headless mode. run `npm install keystone-admin` to install the KeystoneJS
      admin UI.
module.js:328
    throw err;
    ^

Error: Cannot find module 'underscore'
    at Function.Module._resolveFilename (module.js:326:15)
    at Function.Module._load (module.js:277:25)
    at Module.require (module.js:354:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (C:\Users\Carlos\git\prs\pr2390\keystone\lib\schemaPlugins\localize.js:1:71)
    at Module._compile (module.js:410:26)
    at Object.Module._extensions..js (module.js:417:10)
    at Module.load (module.js:344:32)
    at Function.Module._load (module.js:301:12)
    at Module.require (module.js:354:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (C:\Users\Carlos\git\prs\pr2390\keystone\lib\schemaPlugins.js:5:20)
    at Module._compile (module.js:410:26)
    at Object.Module._extensions..js (module.js:417:10)
    at Module.load (module.js:344:32)
    at Function.Module._load (module.js:301:12)

[geloescht]

@webteckie Thanks for trying! Something is weird there in your stack trace. There shouldn't be a file lib\schemaPlugins\localize.js. I seems you somehow pulled in my master branch. There's some old commits about content localization there, which I really really should fix. Better delete your local branch and check out again.
The error message is confusing, I know. I intended to show it only if keystone-admin was missing in a later release after keystone and keystone-admin become separate packages. As it is right now, it shows whenever an exception occurs in keystone-admin and even though keystone-admin is a bundled dependency.

[webteckie]

I followed the PR CLI instructions :-)

[geloescht]

@webteckie Uhm.... and those are? Sorry, I could be more familiar with github. Maybe I can help you on Gitter getting this thing set up (my Slack invite is still not working...). Browser testing is definitely something this PR could benefit from.

[webteckie]

At the bottom of the PR:

[geloescht]

@webteckie Your local master probably points to geloescht/keystone master, but it should point to keystonejs/keystone master. Please do git clone https://github.com/keystonejs/keystone.git to create a clean clone from keystonejs/keystone and from there on follow the instructions.

[webteckie]

@geloescht are you not working off a keystone fork? It seems like you are...btw, I think you got an invite to slack...can you please accept it, if so? we can keep discussing there.

[geloescht]

@JedWatson Any idea why npm install is still failing with a peer dependency error on Travis CI in spite of the fork you made?

[JedWatson]

@geloescht no, I haven't worked that out. It's driving me crazy 😠

[geloescht]

@JedWatson the issues with react-color seem to be fixed. Now, can we have npm v3 on Travis so bundled dependencies get their sub-dependencies installed?

[sjapps]

@JedWatson Any idea when we'll get this merged in?

[bassjacob]

Hi @geloescht, thank you for the massive effort in this pr! Unfortunately we can't see an easy path to getting this merged in, so we're going to close it for now. We're intending to ship this feature in the future, would you be interested in helping us out? Let us know if you're still interested.