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

Module Improvements #10

Open
pi0 opened this Issue Nov 18, 2018 · 9 comments

Comments

Projects
None yet
5 participants
@pi0
Copy link
Member

pi0 commented Nov 18, 2018

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:

  • ModuleContainer is bound to the nuxt runtime (ie, currently running nuxt version). Modules don't know who is running them and what utilities are available for them! This is why after initial version almost nothing new supported in ModuleContainer and instead module authors had to write their own.
  • If a bug is inside ModuleContainer it is not fixable by modules
  • It has not backward/forward compatible utilities
  • It is implicitly injected by bounding module's function into moduleContainer context. Splitting module into smaller logical functions is like a pain in a**!
  • No class support
  • It is optimized and not hackable by people
  • Modules execute every f** time that nuxt starts. Development or Production. 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?)
    • nuxt-start requires all module packages to be installed. Only the built-only ones.

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?

@pi0 pi0 referenced this issue Nov 18, 2018

Closed

Custom CLI Commands #11

@pi0 pi0 added RFC and removed ecosystem enhancement labels Nov 18, 2018

@pi0 pi0 changed the title [RFC] Module Spec Improvements Module Spec Improvements Nov 18, 2018

@pi0 pi0 changed the title Module Spec Improvements Module Improvements Nov 18, 2018

@manniL

This comment has been minimized.

Copy link
Member

manniL commented Nov 18, 2018

Great points 👍

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 🤷‍♂️
We need a more detailed modules guide as well.

@wagerfield

This comment has been minimized.

Copy link

wagerfield commented Feb 5, 2019

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:

  • this.defaultConfig references the default Nuxt config
  • this.projectConfig references the nuxt.config.js (defaults to {} if not found)
  • this.options is the current implementation—the merged result of defaultConfig < projectConfig
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

This comment has been minimized.

Copy link
Member

Atinux commented Feb 5, 2019

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

This comment has been minimized.

Copy link

wagerfield commented Feb 5, 2019

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:

  • path is a string dot separated path to the value to configure eg. "server.host"
  • value is the value to set at that path eg. "0.0.0.0"
  • freeze is a boolean flag that specifies whether or not the module consumer can override this value in their project nuxt.config.js. Defaults to false
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

This comment has been minimized.

Copy link
Member Author

pi0 commented Feb 5, 2019

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

This comment has been minimized.

Copy link

wagerfield commented Feb 5, 2019

@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

This comment has been minimized.

Copy link

pimlie commented Feb 7, 2019

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

This comment has been minimized.

Copy link

wagerfield commented Feb 13, 2019

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

This comment has been minimized.

Copy link
Member Author

pi0 commented Feb 13, 2019

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

  • Meta-definition should be more mature. Maybe we can explicitly define module dependencies too.
  • Extends should be a part of config package spec. (Like a built-in functionality) but that's right. The ability to define extends from a module should be considered: Question is option override order.
  • The multi-source (Like multi-root keys in options in PWA example) should be considered
  • Easier hooking into nuxt using nuxt hooks system. (Actually, it is going to be to hable integration)

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.