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

Monorepo plugins #292

Closed
wants to merge 58 commits into from
Closed

Monorepo plugins #292

wants to merge 58 commits into from

Conversation

loklaan
Copy link
Collaborator

@loklaan loklaan commented Apr 4, 2017

This PR tracks the progress of moving nwb and it's plugins into a monorepo, as well as implementing related issues.

Progresses #250, but doesn't resolve it.

  • Bootstrap monorepo structure with lerna
  • Move nwb into a package in monorepo
  • Fix CI concerns. eg TravisCI / Appveyor
  • Move existing plugins into monorepo:
    • nwb-less
    • nwb-sass
    • nwb-stylus
  • Extract Karma test runner into a plugin
    • Plugin architecture needs to be redesigned to be dynamic
    • A number of pure utility functions needs to be pulled into a seperate package, to share with plugins. eg createWebpackConfig
    • Test (likely need to integration tests from in the plugin)
  • Add plugin with Jest test runner Add Jest support #173
    • Validate that the plugin arch works well for Jest too
  • Update existing plugins with new plugin architecture, if applicable
  • Try to make CI errors easier to read (lerna seems to obfuscate test errors, but perhaps too early to make that statement)

Plugin Architecture

Going forward, plugins are going to fit more cases:

  • Provide nwb configs. eg existing plugins
  • Projects with build, serve, and other commands. eg react-app, web-module, vuejs...
  • One-off commands that use the final nwb configuration. eg Karma/Jest runners, using nwb configs
  • Targeted partial templates. eg Templates for Karma/Jest tests

Plugin API Proposal

Exporting an object of a nwb config should still be supported.

To support the other cases, a plugin package could also export a function:

// Plugins can register configuration as a:
//
// * Project
//    register.project
//
// * CLI Command
//    register.cliCommand
//
// * Targeted template
//    register.targetedTemplate

module.exports = function (register) {

  register.project({
    name: 'vue',
    description: 'VueJS app',
    template: '../project-template',
    commands: {
      // Special commands are given webpack helpers to call
      build: function (cliArgs, build) {
        // Create a partial nwb config
        build(nwbConfig)
      },
      serve: function (cliArgs, serve) {
        // Create a partial nwb config
        serve(nwbConfig)
      },

      // Other commands have API like register.cliCommand
      clean: { /* ... */ }
    }
  })

  register.cliCommand({
    name: 'test',
    args: [
      // Example only
      //[ 'entry', 'entry point', 'src/index.js' ]
    ],
    opts: [
      [ 'coverage', 'create a code coverage report' ]
    ],
    callback: function (cliArgs, nwbConfig, doneCb) {
      // doneCb must be called to exit cleanly
    }
  })

  // Project templates can contain files with a type as the name. It will be
  // replaced by the directory/file at './template'
  register.targetedTemplate({
    target: register.testTemplate.TYPES.TEST_AT_ROOT,
    template: './template'
  })

}

A lot of the nwb help, for projects and one-off cli commands, can be inferred from properties of this api.

Note that the build/serve commands for a project aren't provided a nwb config; none of the existing projects need one before given their partial config to webpackServe or webpackBuild, so it was left out of the api.

insin added 30 commits February 13, 2017 10:21
--preact and --inferno are really aliases for these
glob and rimraf are now only devDependencies.
- Don't move `loader` config into an `options` object
- Validate that rule `use` config is an Array
- Add a `path` property to user config when sucessfully loaded
- Default rules can now be disabled by configuring them as false
- Default rule loader and options are now omitted if you configure `loader` or `use`
- Add a separate factory for loaders, which only handles loader+options
- Add support for a webpack.config() function which receives the generated config to be edited and returned.
[ci skip]
[ci skip]
…s can be used in user config

e.g. if adding a new rule for a file type not handled by default loaders. 'url-loader' can be used
Initial commit.
@loklaan
Copy link
Collaborator Author

loklaan commented Apr 4, 2017

Dug deeper into the requirements of the dynamic side of a nwb plugin arch. I've updated the PR summary to reflect the requirements, namely the api proposal code example.

@insin Could I get some feedback if you got some time? I've gathered that you had some ideas about the API from before. It may be a bit involved to give help, sorry, but it'd be goooood before I jump into implementing 💥

@insin insin force-pushed the next branch 4 times, most recently from 4145b32 to 8d700e3 Compare May 19, 2017 11:33
@insin
Copy link
Owner

insin commented Jun 1, 2017

Monorepo looks 👌

How painful will it be to call a halt to development at some point to get it rebased against master?

I've avoided plugin abstractions so far because you don't know if you've messed up until you've really dug into implementation. Will have some time tomorrow to have a proper look.

@loklaan
Copy link
Collaborator Author

loklaan commented Jun 3, 2017

No I get that - early optimisation.

It wouldn't be so bad, but it'd likely be cleaner to replay my steps from the beginning with newest master.

  1. Move package into subdir packages/nwb
  2. Copy the root dir's package.json and lerna.json from this PR, because they have already tackled CI problems afair
  3. Use lerna to import git histories of the styling plugins
  4. ...I think that's all the steps.

@insin insin force-pushed the next branch 8 times, most recently from 2909ed8 to c21739b Compare June 13, 2017 17:44
@insin
Copy link
Owner

insin commented Jun 14, 2017

Moving towards being able to register commands in next instead of just using module names in the filesystem.

The build-${type}[-app].js and serve-${type}[-app].js commands are now just calling the same functions and passing a different config module in.

@insin insin mentioned this pull request Jun 22, 2017
@loklaan loklaan mentioned this pull request Sep 17, 2017
@loklaan
Copy link
Collaborator Author

loklaan commented Sep 17, 2017

I'm jumping off of this issue. I'll leave it to insin to close, since it might be valuable for posterity.

@loklaan loklaan closed this Nov 16, 2018
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.

3 participants