nuxt / rfcs

RFCs for changes to Nuxt.js
99 stars 2 forks source link

Module Improvements #10

Open pi0 opened 6 years ago

pi0 commented 6 years ago

TLDR

A powerful engine is not usable without a good interface. Nuxt modular refactor made a long time ago but it is still lacking a good SDK.

History

The initial proposal for modules started with a project called nuxt-helpers, a wrapper function around export inside nuxt.config.js. Nuxt was hackable enough to inject any parts of it. But we wanted a more stable and official way to access and modify Nuxt internals and hack things beyond webpack extend. Nuxt core has been broken into smaller modules with their specific tasks that inherit a global options object and communicate with each using Nuxt class (The hub). Then we needed to create an entry point for modules that can start hacking Nuxt from zero to build and start. We thought about different ways like an exported object that contains strict options but it was against the creativity of module authors. With a simple function, module author is free to get the nuxt instance and hook-up into any part of built-ins. Actually, this pattern is powerful more than enough. I've never thought we should change it. Modules like nuxt7 built on top of this simple pattern can customize anything!

So the problem

Modules, while being free to touch any internal by using nuxt reference should be somehow isolated not only because of providing them utilities like addPlugin() but also because nuxt internals (or it's dependencies like webpack 3 => webpack 4) may be changed at any time. The bad decision (most of mine) was using ModuleContainer for utility container. This has several disadvantages:

The solution

The module utilities (SDK) should be provided as a separated package (Maybe even a dedicated repository in nuxt-community). Other than regular utilities this package can provide a HOC (wrapper function) and Base class that finally evaluates into a plain function usable by Nuxt engine. This guarantees both old and new modules can work with old and new versions of Nuxt while module authors are just encouraged to use SDK instead of using legacy ModuleContainer helpers. Module authors can create their HOC functions too.

Usage

With class

import { NuxtModule } from '@nuxt/module'

class MyModule extends NuxtModule {
  static get key() {
    return 'myModule' 
  }

  // This mapping can be moved into base class too
  static get hooks() {
    return {
      'nuxt:init': 'onNuxtInit',
      'bulder:build': 'onBuild'
    }
  }

  async onBuild() {
    this.addPlugin(...)
  }

  async onNuxtInit() {
    this.nuxt.render = (_, res) => res.end('šŸŒˆ')
  }
}

// Compile class into a plain function
export default MyModule.export()

With wrapper function

import { createModule, addPlugin } from '@nuxt/module'

export default createModule({
  name: 'MyModule',
  key: 'myModule',
  hooks: {
    'nuxt:init'(nuxt) {
       this.nuxt.render = (_, res) => res.end('šŸŒˆ')
    },
    'builder:build'(nuxt, builder) {
        addPlugin(nuxt, ...)
    }
  }
})

Personally, I like the wrapper pattern as it is closer to the Vue exports, is simpler to write (no export() call) and most importantly it is much better for implementing lazy-requires that are basically context-less pure functions. But we may implement both :)

Chore

  1. What do we call this package? @nuxt/sdk | @nuxt/module | @nuxt/module-utilities ?
  2. It should be a part of the monorepo or separate repository?
  3. Should we keep old ModuleContainer codes or move them to the new SDK and depend @nuxt/core on the latest version of it?
manniL commented 6 years ago

Great points :+1:

Having a dedicated utility repo/package for modules is definitely a must-have and will make module development even easier.

My thoughts:

Class vs. Function

I'd stick to the create function (personal preference). Would vote against having both approaches/ways built-in because it'll "split" the module developers and doesn't add any real value. But concerning this, it'd make sense to talk to the module maintainers as well so we all can decide on one way to do it.

Package name

@nuxt/module-utilities sounds pretty good to me. @nuxt/modules is too brief IMO and @nuxt/sdk sounds more like a developer tool for the core to me.

Place of the module

Probably an own dedicated repo (maybe even under the nuxt namespace?).

Old ModuleContainer code

We can't remove it for now as it'd be a breaking change, but for 3.X I'd do that. People should start transitioning as soon as it's out so we can remove the code. A "bridge" for older modules might be an idea worth to pursue as well.

Question of Scope

The utilities are 'only' for modules (not for replacing, say @nuxt/webpack with an own implementation of @nuxt/parcel), or should they cover that as well?

Misc

Actually, module authors can use this.nuxt.hook guard to lazy require and only configure webpack options when we are building project but in reality, no-one did this (Maybe because it was hard?)

Likely because it wasn't written down anywhere / suggested to do so :man_shrugging: We need a more detailed modules guide as well.

wagerfield commented 5 years ago

I'm hoping this is the correct issue to add my module feature request to...

I would like to see a couple of additional properties added to the module container scope that allow module authors to access both the default Nuxt config and the project nuxt.config.js if provided.

Right now the only way of accessing the config is via this.options. The problem with this.options is that it is the result of merging the default Nuxt config with the project nuxt.config.js file.

As a module author, I would like to be able to override some of the default configuration of Nuxt while still allowing consumers of my module to override them via their project nuxt.config.js file.

For example, I continuously find myself setting server.host: "0.0.0.0" so that I can access my app on another device on the same network. I also like to set the srcDir to "src" and override the router link classes so my nuxt.config.js always contains these overrides:

// nuxt.config.js
export default {
  srcDir: "src",
  router: {
    linkActiveClass: "link-active",
    linkExactActiveClass: "link-active-exact"
  },
  server: {
    host: "0.0.0.0"
  }
}

Since I find myself doing this on every project, I decided to create a module that sets these values by default:

export default function SomeModule() {
  this.options.srcDir = "src"
  this.options.server.host = "0.0.0.0"
  this.options.router.linkActiveClass = "link-active"
  this.options.router.linkExactActiveClass = "link-active-exact"
}

The problem with this approach is obvious: you are not able to override any of these values via a project's nuxt.config.js file:

export default {
  modules: ["some-module"],
  server: {
    host: "127.0.0.1" // will be overridden by "some-module"
  }
}

This presents a problem for option merging strategies within the module container scope since this.options is the result of merging the default Nuxt config with the project config. For modules that want to override default Nuxt config values while still allowing project configs to override them further, there isn't a simple way of doing this currently.

I posted this question in the Discord channel and @manniL kindly responded with a solution using the @nuxtjs/config package to getDefaultNuxtConfig. Though this works, I would like to see both the default Nuxt config and the project config available within the Module container scope alongside the options to make config merging strategies simpler for module authors:

export default function SomeModule() {
  this.options.srcDir = this.projectConfig.src || "src"

  Object.assign(this.options.router, {
    linkActiveClass: "link-active",
    linkExactActiveClass: "link-active-exact"
  }, this.projectConfig.router)

  Object.assign(this.options.server, {
    host: "0.0.0.0"
  }, this.projectConfig.server)
}

Feedback welcome šŸ˜ƒ

atinux commented 5 years ago

Really nice suggestion @wagerfield

I guess this could be a possibility to export something like a config(projectConfig, nuxtConfig) method where you could overwrite only projectConfig and set default if not defined or overwrite default nuxtConfig (and then we actually merge projectConfig & nuxtConfig in Nuxt directly)

wagerfield commented 5 years ago

Thanks @Atinux,

Perhaps a config merging function made available within the Module Container would be a nice solution. This approach would give Nuxt control over merging special keys like build.extend or anything like that where the values are functions for example.

Whatever the solution isā€”it would have to be flexible enough to allow module authors to granularly control the config merging strategies for different keys.

For example, you might want to override some part of the default Nuxt config in your module and not allow consumers of the module to override it in their project config...while in the same module override the default Nuxt config and allow module consumers to override it themselves if desired.

Perhaps the function signature of the config function could be as simple as:

config(path, value, freeze)

Where:

export default function SomeModule() {
  // Can be overridden in nuxt.config.js since freeze is omitted
  this.config("srcDir", "src")

  // Can also be overridden in nuxt.config.js
  // Notice that an object is passed here with key values
  this.config("server", {
    https: true,
    port: 8080
  })

  // Cannot be overridden in nuxt.config.js since freeze is true
  this.config("server.host", "0.0.0.0", true)

  // Cannot be overridden in nuxt.config.js
  this.config("router", {
    linkActiveClass: "link-active",
    linkExactActiveClass: "link-active-exact"
  }, true)
}

To you suggestion above, if there were to be any named keys or param names in a config function, I would suggest using defaultConfig over nuxtConfig since this might be a little ambiguous due to the projectConfig being the nuxt.config.js file.

pi0 commented 5 years ago

Nice idea about config overriding/preset @wagerfield. But config handling is out of module scope. The order of a nuxt project bootstrap is:

1.CLI > 2.Read Config > 3.Normalize Options > 4.Run Modules > 5.Ready (Including Listen, ...)

All current modules assume that shape of this.options is already normalized with applied defaults. As module entry point function is exactly one, the only possible way would be swapping 3 with 4 which is not only a breaking change but makes modules unstable depending on user input.

I suggest moving this enhancement to #16 (Improve Config) by allowing presets in nuxt.config:

{
  presets: [
     'nework',
     '@company/defaults'
  ]
}

Presets can have an interface just like modules a function like you described above.

PS: As described in original RFC I suggest keeping ModuleContainer frozen and instead introduce module utils package.

wagerfield commented 5 years ago

@pi0 having just looked at the source code, I can see the issue in my proposal above.

I agree that having a presets or extends key with an array of strings that resolve to files exporting a Nuxt config is the way to go. Nice idea šŸ‘

I'll add that as a comment to #16 as per your suggestion.

pimlie commented 5 years ago

With regards to Class vs Function, I dont mind either or both as long we dont have to make any knee falls for them in regards to usability. This is in reference to the current NuxtCommand implementation in @nuxt/cli which isnt optimal imo because the this scope in the actual (plain object) commands are different from the this scope in the NuxtCommand class which greatly impacts the hackability.

The name @nuxt/module-utilities doesnt sound like a building block for creating a module to me, sounds more like some optional helper functions. Maybe a combination of the ones mentioned above? @nuxt/module-sdk or @nuxt/module-runtime makes it almost immediately clear for me as a developer that its likely I need to have that as a dependency in my project when I want to create a module.

wagerfield commented 5 years ago

Having read over everyone's comments again, here's are some more thoughts on this...

I think the new module SDK/API should live in its own package within the monorepoā€”not in a separate repo. It's ultimately apart of Nuxt, so it makes the most sense to me for it to live alongside the other packagesā€”making it easier to test and update the docs, change log etc. with each new version.

Of the package suggestions from @pi0 I like @nuxt/module the most. It sits nicely alongside @nuxt/core, @nuxt/config etc. The key for registering modules in the Nuxt config is modules so I think @nuxt/module is an appropriate package name to import utils from.

I also prefer the wrapper function over extending a class and agree with @manniL that you should only be able to do one in order to maintain consistency in modules authored by the community and the team.

Having said that, do we need to wrap the module options in a function at all? Could you not just follow suit of Vue component options and simply export an object that Nuxt then wraps during setup when modules are registered in the Nuxt config?

Here's my proposal extending from your earlier one @pi0:

import { addPlugin, addModule } from '@nuxt/module'
import pkg from './package.json'

const moduleConfig = {
  router: {
    linkActiveClass: "link-active",
    linkExactActiveClass: "link-active-exact"
  },
  server: {
    host: "0.0.0.0"
  }
}

export default {
  name: 'PWA', // required
  meta: pkg, // required

  // Could be a String or String[] to allow modules
  // to register multiple options keys (like the PWA module)
  options: ['icon', 'manifest', 'meta', 'workbox'], // String[]
  options: 'sitemap', // String

  // Extend a single Nuxt config or an array of Nuxt configs
  // Useful for creating modules that configure Nuxt a certain way
  // Can be an plain config object that is declared locally or imported
  // Or a string path that uses node's resolver to require it
  // Or an array of objects and string paths
  extends: ["./config", "some-nuxt-config-preset", moduleConfig],

  hooks: {
    nuxt: {
      init(nuxt) {
        // 'this' would be bound to the result of 'wrapping' the module object
        // eg. this === createModule(module)
        // where 'createModule' is called by Nuxt during setup
        // and 'module' is this module definition object
        console.log(this) // { name: 'PWA', options... }

        // this.options would return a 'resolved' object with just the module options (if set)
        // if options is a string it will return the options value assigned to it
        // if options is an array of strings it will return an object with each of those key values
        console.log(this.options) // { icon: undefined, manifest: { ... }, meta: { ... }, workbox: false }

        // nuxt.config would return the loaded Nuxt config, undefined otherwise
        console.log(nuxt.config) // { srcDir: "src", css: ["normalize.css"] }

        // nuxt.options would return what this.options does currently
        console.log(nuxt.options) // { srcDir: "src", css: ["normalize.css"], icon: {}, manifest: {} ... }
      }
    },
    build: {
      before(nuxt, builder) {
        addPlugin(nuxt, './plugin.js', { /* options */ })
        addModule(nuxt, './module.js', { /* options */ })
      }
    }
  }
}

I have expanded the hooks into nested functions in the same way that they are documented here:

https://nuxtjs.org/api/configuration-hooks

Personally I prefer this as I think it's cleaner than "nuxt:init"() {}...but perhaps Nuxt could support both?

To collate all that has been discussed so far, here is a table of keys that could serve as some initial documentation of the Module object API:

Key Type Description
name String Name of the module. Required.
meta Object Meta data for the module. Required.
options String|String[] Options key(s) for the module within the Nuxt config.
extends String|Object|Array Extend the default Nuxt config. Options can still be overridden by the user in their Nuxt config.
hooks Object Nuxt hooks to bind to. Can use the format 'nuxt:init'() {} or nuxt: { init() {} }
pi0 commented 5 years ago

@wagerfield Nice write-up. Thanks. I appreciate it. Points I can extract from it as TODO for RFC:

Regardin the place of @nuxt/module in the mono-repo there are two big concerns:

  1. Versioning of mono-repo is dependent. This may make issues for module authors that require independent versioning for utils.
  2. It is actually a variant of (1) but is really important. Module Utils should be backward/forward compatible. This is more than testing against a single (current) version of nuxt. A build matrix against supported nuxt versions would be required.