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

[RFC] Project Restructure #986

Closed
wants to merge 2 commits into from
Closed

[RFC] Project Restructure #986

wants to merge 2 commits into from

Conversation

obfu5c8
Copy link

@obfu5c8 obfu5c8 commented Dec 31, 2017

Project Restructure RFC

Proposal to heavily reorganise the project structure to separate the concerns of different parts of the system.


Introduction

I've been trying to find a good es6 documentation generator for a while. I like documentation a lot, but it's not quite flexible enough to do everything i'd like. yet.

I thought i'd share my vision for how i think a more decoupled codebase might look - a proper separation of concerns would make make iterating features much easier (and safer) as well as opening the door for a plugin ecosystem (implementing plugins was what got me thinking about this restructure in the first place).

This PR is by no means ready for integration at this point. It is shared here more to exemplify my ideas and to start a conversation on the topic. Many of the usage examples below are not yet implemented - I wanted to see how the RFC was received before I committed too much time to it.

Separation of Concerns

I've tried to separate out concerns as much as possible into different packages that can (if desired) be imported separately. This means that end-users can use as much, or as little, of documentation's code as they wish.

Package Overviews

documentation-core

This package contains the core nodejs API for parsing comments and creating formatted outputs with them. The majority of the existing documentation codebase lives in this package.

It provides no CLI or server options, and is very low-level, requiring fine-grained configuration to use. This is for advanced users who wish to do funky things with their doc generator.

Example usage
const DocumentationEngine = require('documentation-core/engine');
const ThemeOutput = require('documentation-core/output/theme');
const DefaultTheme = require('documentation-theme-basic');

const MyPlugin = require('./my/plugin');
const MyCommentParser = require('./my/parser');


const docs = new DocumentationEngine()

// Register a plugin that this engine instance will use
docs.use(MyPlugin);

// Add comment parsers directly to the parsing pipeline
docs.pipeline.add(MyCommentParser)

// Parse sourcecode and generate HTML output using the specified theme
docs.build(['index.js'], config)
.then(comments => 
    docs.output(comments, ThemeOutput(DefaultTheme), config)

documentation-cli

Contains a CLI wrapper for the core API - At the moment I have simply extracted the existing CLI from the documentation package.

(Also, it needs to be updated so that it builds and passes a tidily structured config object to the core API, rather than polluting the DocumentationConfig object with all sorts of yargs argv crap </rant>)

Depends on:

  • documentation-core
  • documentation-theme-basic

documentation-theme-basic

Contains the default HTML theme for formatted output. Has been extracted wholesale from the existing documentation codebase.

documentation

This package would replace the existing documentation package and serve as a thin wrapper to bring together the decoupled parts while maintaining backwards-compatability. This makes it really easy to consume in normal use cases.

Depends On:

  • documentation-core
  • documentation-cli
  • documentation-theme-basic
Example implementation
const DocumentationEngine = require('documentation-core/engine');
module.exports = new DocumentationEngine();

@tmcw
Copy link
Member

tmcw commented Jan 2, 2018

WOW. This is amazing: thanks for submitting this RFC, @obfu5c8! cc @anthony-redFox @arv @anandthakker to take a look at this.

Re: the changes -

  • In this longish history of this project, there used to be a similar separation between the default theme and documentation.js itself - which I reversed in this current iteration of the project, which is quite 'monolithic'. The main reasons for this unification were:
    • If there's a A → B relationship between documentation & a theme, I want themes to be relatively 'free' of documentation.js dependencies. This was originally 'solved' by adding documentation-theme-utils, another util that we could rely on. In this PR, the default documentation theme requires documentation-core. Neither of these solutions feel right to me - like someone might have a theme that requires documentation-core of one version and has a cli with another version and the two get twisted. Also if we're picturing themes as lightweight transformations of an intermediate output, it doesn't work great that they'd require documentation-core itself. I think this is mostly unnecessary - I was super... optimistic/idealistic about making the output super generic, hence how documentation produces a markdown ast instead of transforming to HTML itself, or even passing markdown. I think documentation-core itself should do the interlinking, thus allowing us to drop the dependency. (Note the dx-spec conversation about this)
  • The connection between the yargs-ish stuff and the documentationconfig object has been tricky: yargs has its own validation layer, and then we also want misuse of the documentation-core API to be similarly guarded about types and values. I wonder if separating out a CLI means that maybe we should only do validation in the documentation-core layer and make yargs permissive? I'm not sure about this but it's one of the major cross-cutting concerns that I'd expect to be exacerbated by a modularization effort.

Overall, really excellent work, and interested to hear what the other contributors have to say!

@obfu5c8
Copy link
Author

obfu5c8 commented Jan 2, 2018

Themes

I 100% agree about removing dependencies from the themes - I think they exist in this commit because i hadn't yet managed to extricate the shared functions from the core.

I've been pondering today whether the format vs theme thing couldn't be simplified as well - after all they do the same job, but with different outputs. I would propose that an "output formatter" should simply be a function that takes an array of Comment as input (and optionally some configuration) and returns an array of File (vinyl) as output. That way there is no distinction between types of output.

Also, i think that separating file generation and output would be beneficial as it would allow the possibility of adding other types of Output handlers (CLIOutput, GithubPagesUpload perhaps?)

Concept

const docs = new DocumentationEngine();

docs.parse(['index.js'], config)
.then( comments => docs.format(comments, MarkdownFormatter))
.then( files => docs.output( files, FilesystemOutputter ))

Concept CLI implementation

let formatter;
switch(argv.format) {
    case 'json':
        formatter = JsonFormatter;
        break;
    case 'markdown':
        formatter = MarkdownFormatter;
        break;
    case 'html':
        formatter = require('documentation-theme-default');
        break;
    default:
        // allow loading any node module as a formatter
        formatter = require(argv.format);
}

docs.format(comments, formatter) /* ... */

CLI yargs validation

I feel that the CLI should only be responsible for very basic validation of the arguments as they are received - only enough to populate a DocumentationConfig object which is then passed to the core to be processed. If the core then decides the configuration is invalid it should throw an exception that the CLI can catch and present pleasantly to the user.

Next Steps

I'll try and find time in the next day or two to push up a couple more commits to show the separation process so far, hopefully it will better illustrate my thinking in these areas

@obfu5c8
Copy link
Author

obfu5c8 commented Jan 3, 2018

OK so i've got the most basic functionality back in place again.

$> cd ${project_root}
$> yarn run build
$> yarn documentation build ./packages/core/src/engine.js --format markdown

you can also write the markdown to a folder with --output-dir <dir> argument

Still to do

  • Refactor HTML formatter to be standalone, depending only on @documentation/format-utils
  • Validate JSON formatter works as expected
  • Extricate the buildPipeline comment parsing stack from the core engine class
  • Finish implementing plugin framework
  • Properly implement all CLI arguments
  • Tests, tests, tests
  • Check for redundant dependencies

@obfu5c8 obfu5c8 closed this by deleting the head repository Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants