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

Improve Config Package #16

Open
pi0 opened this Issue Dec 11, 2018 · 18 comments

Comments

Projects
None yet
6 participants
@pi0
Copy link
Member

pi0 commented Dec 11, 2018

Objectives:

  • Rewrite in typescript
    • Requirement: Rollup TS Support + Dev script workaround
  • A better namespace for options of each module
  • Moving each config to its own package (Builder, Core, etc)
    • Injectable config interface
    • Split options.js
  • nuxt.config validation (Both type and keys) and IDE autocomplete
  • Prepare for Nuxt UI for visual config changes
  • Ability to persist some of the options after build in .nuxt dir (The Initial version made by @clarkdo but currently disabled). This is essential for improving the size of nuxt serverless builds.
  • Import aliases in nuxt.config.js (nuxt/nuxt.js#4838 - @wagerfield)
  • Supporting config/ directory and importing the files and merging them with nuxt.config.js (that could be deprecated in the future), see @Atinux's comment

Related: nuxt/nuxt.js#3985

@pi0 pi0 added nuxt RFC labels Dec 11, 2018

@nuxt nuxt locked and limited conversation to collaborators Dec 11, 2018

@nuxt nuxt unlocked this conversation Dec 11, 2018

@pi0 pi0 removed the RFC label Dec 11, 2018

@manniL

This comment has been minimized.

Copy link
Member

manniL commented Dec 11, 2018

nuxt.config validation (Both type and keys) and IDE autocomplete

Might be hard (for now) because of modules which can freely choose their used keys.
We can validate the core keys without problems though.

@pi0

This comment has been minimized.

Copy link
Member Author

pi0 commented Dec 11, 2018

@manniL Exactly. At least the top level object cannot be strictly validated. We might add a special __validate__ key to the options that are validatable. If we use a well-defined object for schema definition we can magically use it for automated .d.ts generation too.

Example:

{ 
 __validate__: {
       _strict: false, // <-- Allow modules freely adding new keys for BW compat
       build: { type: Object }
 },
 build: {
     __validate__: {
        _strict: true, // <-- Indicates no extrra options are accepted
        name: { type: String, required: false },
        old: { type: String, deprecated: true }
    }
 }
}

We can also think about using a 3rd party schema validator that is able to generate types too. Like joi + joi-ts-generator

@pi0 pi0 referenced this issue Dec 13, 2018

Closed

feat: add typescript support + an example #4406

1 of 3 tasks complete

@galvez galvez referenced this issue Dec 20, 2018

Open

Fabula #20

@Atinux

This comment has been minimized.

Copy link
Member

Atinux commented Dec 27, 2018

Supporting config/ directory and importing the files and merging them with nuxt.config.js (that could be deprecated in the future).

Example:

config/
  build/
    babel.js
  pages.js
  app.js

This logic could use directly glob right inside @nuxt/config. So writing configuration could also feel like writing pages <3

@kevinmarrec

This comment has been minimized.

Copy link
Member

kevinmarrec commented Dec 27, 2018

@Atinux Nice, I really liked how @manniL went with his own website with a config folder : https://github.com/manniL/lichter.io/tree/master/config. It seems to go in the same way of your config directory support

@wagerfield

This comment has been minimized.

Copy link

wagerfield commented Jan 23, 2019

I would be wary of using a directory called config since there is already a popular package called config that reads JSON files from this directory at the root of your project:

https://www.npmjs.com/package/config

I've used the config package a number of times in Nuxt projects and it might get a little muddled if there are Nuxt related config JS files alongside static JSON config files.

Perhaps nuxt is a better default? Appreciate that Nuxt generates a .nuxt dir for dev and build, but nuxt without the . could be for config. Funnily enough, this is the convention that I have adopted for a recent project:

// nuxt.config.js
import generate from "./nuxt/generate"
import hooks from "./nuxt/hooks"
import head from "./nuxt/head"
import env from "./nuxt/env"

export default {
  srcDir: "src",
  generate,
  hooks,
  head,
  env
}
@manniL

This comment has been minimized.

Copy link
Member

manniL commented Jan 23, 2019

@wagerfield If we implement that feature, the directory name will be changeable ☺️
But thanks for the heads up!

@manniL

This comment has been minimized.

Copy link
Member

manniL commented Jan 23, 2019

Another thing that might come in handy:

A context object which can/will be passed to each sub config and can be set by the user. This might help to reduce async calls to APIs during build (e.g. when you want to define your routes for the sitemap, the blog feed and generate).

@kevinmarrec

This comment has been minimized.

Copy link
Member

kevinmarrec commented Jan 23, 2019

@manniL context could be of type NuxtConfiguration in TypeScript environments: https://github.com/kevinmarrec/nuxt.js/blob/nuxt-configuration-types/packages/config/types/index.d.ts#L18

EDIT : might have misunderstood context proposal

@manniL

This comment has been minimized.

Copy link
Member

manniL commented Jan 23, 2019

@kevinmarrec I thought about sth. like an argument that every sub-function could take (if it's written as function)

so the subconfigs would be sth. like export default (context) => { /* Do sth. here */ }

I saw that @Timkor (hope that was the right handle 🙈) applied such a pattern:

const context = require('./shared/utils/context');

context.print();

module.exports = {
    
    server: require('./config/server.js')(context),

    env: require('./config/env.js')(context),
    /*
     ** Headers of the page
     */
    head: require('./config/head.js')(context),

    // Sitemap settings:
    sitemap: require('./config/sitemap.js')(context)
}
@kevinmarrec

This comment has been minimized.

Copy link
Member

kevinmarrec commented Jan 23, 2019

Oh yes right ! I'm now thinking of a new behavior for subconfigs 🤔

What about

// config/build.js
export default function (config, context) {
  config.build = {
    property: context.value
  }

  return config
}

The idea is to read every files in config folder and merge the values to build the final configuration file. People could preprend their filenames with numbers in cases they need to override same configuration properties in certain order in different files (rare case but could happen)

EDIT : It would work likely like Webpack extend config

@wagerfield

This comment has been minimized.

Copy link

wagerfield commented Feb 5, 2019

Further to the discussion in #10 @pi0 suggested adding a presets field to the Nuxt config that would work in the same way as Babel. As an alternative, I prefer ESLint's extends field and behaviour for this use case since it supports a single string and an array of strings. I also think extends is more semantically correct 🤷‍♂️

The value assigned to this field could either be a string or an array of strings that resolve to files exporting a Nuxt config:

// nuxt.config.js
export default {
  // String value resolving to "some-nuxt-preset" module exporting a Nuxt config
  extends: "some-nuxt-preset"

  // Can also be an array of strings, merging these presets in order
  extends: [
    "some-nuxt-preset",
    "another-nuxt-package/config"
  ]
}
@pi0

This comment has been minimized.

Copy link
Member Author

pi0 commented Feb 5, 2019

@wagerfield I like the extends more 👍

@Timkor

This comment has been minimized.

Copy link

Timkor commented Feb 7, 2019

@manniL Almost right . :)
I like to build the configuration using functions that accept a context parameter. Context is in my cases just based on some environment variables.

Which could also have been just the env key of the configuration. If Nuxt keeps track of a (configurable) order in which to load the configuration files, it would be possible to export a function like this:

~/config/env.js <-- first one by default

export default (config) => {
    return {
        locale: 'en-US'
    };
}

~/config/head.js

export default ({env}) => {
    // We can use env in here, to generate the config of head
}

This way we might also be able to determine the order by the destructuring parameter (like dependencies). But this might be overkill.

Also possible for modules and their options. But here is the order somewhat more important. So you will need a config/modules/index.js to specify them:

export default ({env}, load) => {
    return [
         '@nuxtjs/sentry',
         [
             '@nuxtjs/axios',
             load('~/config/modules/axios')
         ]
    ]
}

There could be a second parameter which takes care of the loading, which just calls the default export with the current configuration as parameter. Similair to plugins.

I think this is easier for new Nuxt.js users than the extend.

@pi0

This comment has been minimized.

Copy link
Member Author

pi0 commented Feb 7, 2019

Plus loading order, we can support Unix style config naming to specify priority be prefixing with numbers. Like 1-env.js / etc.

@Timkor

This comment has been minimized.

Copy link

Timkor commented Feb 7, 2019

@pi0 Actually I think it might also be possible to dynamically determine the order using a Proxy. Something like this:

function makeProxy(config) {
    return new Proxy({}, {

        get: function(obj, prop) {

            if (prop in obj) {
                return obj[prop];
            } else {
            
                console.log('Load: ' + prop);

                return obj[prop] = loadConfiguration(prop);
            }
        }
    });
}


var configuration = makeProxy({});

function loadConfiguration(prop) {
    return require('~config/' + prop)(configuration);
}

Of course you'd need to watch out and warn for circular deps.

@Timkor

This comment has been minimized.

Copy link

Timkor commented Mar 21, 2019

I made a Nuxt module to demonstrate a working POC of dynamic load order of the config files I suggested using a Proxy:

https://github.com/Timkor/nuxt-config

I am curious what you guys think, and if it is a possibility to add this functionality to nuxt.

nuxt-config

Nuxt Module for splitting your nuxt.config.js into multiple files.

Features

  • File based Nuxt configuration.
  • Support for exported Object, Array or Function (accepting current config).
  • Dynamic ordering of executing exported functions using a Proxy.
  • Modules in ~/config/modules.js will be added to Nuxt dynamically.

Examples:

~/config/env.js:

// Support for object
export default {
    quickBuild: true,
    sentryDSN: '...'
}

~/config/build.js:

// Support for function:
// Because of {env} this module will first import ~/config/env.js
export default ({env}) => {
    
    
    return {
        
        hardSource: env.quickBuild,

        extend(config, ctx) {
            
        }
    }
}

~/config/plugins.js:

// Support for Arrays
export default [
  '~/plugins/google-analytics.js'
]

Known issues

There is still one problem: circulair dependencies. If you would have two config functions:

~/config/foo.js:

export default (config) => {
    
    // Read bar
    const bar = config.bar; // This will trigger to import ~/config/bar.js
}

~/config/bar.js:

export default (config) => {
    
    // Read foo
    const foo = config.foo; // This will trigger to import ~/config/foo.js
}

This is not possible. So what happens now is:

  • ~/config/bar.js is executed
    • config.foo is read
    • This triggers ~/config/foo.js
      • config.bar is read
      • Circulair dependency is detected and a warning is thrown.
      • foo continues and config.bar is still undefined
    • config.foo is equal to the result of ~/config/foo.js
    • ~/config/bar.js is continued.

What needs to be done:

The circulair dependency warning is now:

Can not early load '~/config/bar.js' because of a circulair dependency

It would be nice to have a descriptive warning which tells:

  • what is wrong
  • how it can be fixed

An example of a better warning can be:

Can not load '~/config/bar.js' because of a circulair dependency:
 - config.bar is referenced in ~/config/foo.js
 - config.foo is referenced in ~/config/bar.js (resolved)

Some info about how to resolve/fix this.

This can also occur with a path which is larger than 2, for instance: a -> b -> c -> a.

This can be achieved by implementing a dependency tree.

@Atinux

This comment has been minimized.

Copy link
Member

Atinux commented Apr 1, 2019

Hi @Timkor

Thank you so much for investigating in it and creating a module 👍

Actually, I am trying to understand the use case of exporting a method and receiving the config as parameter. What is the real use case?

Actually, for env, I don't know if we should keep it, people can use process.env right inside their config files to conditionally update their config before exporting.

Would like some feedback from @nuxt/core-team

@Atinux Atinux added nuxt 3 and removed nuxt labels Apr 1, 2019

@Timkor

This comment has been minimized.

Copy link

Timkor commented Apr 20, 2019

@Atinux
It was interresting to develop. Though not really neccesary in any way.

I thought it would be nice to somewhere have a definition of the environment variables for better IDE support and better TypeScript integration. In addition, I could think of some use cases where you would want some configuration file dependant on some other properties of the nuxt.config.js. However, those use cases could be solved one way or another.

Edit:
A good use case for this functionality would be for instance:

Generating a sitemap with localisation with the following modules:
https://github.com/nuxt-community/sitemap-module
https://github.com/nuxt-community/nuxt-i18n

It would be nice to filter the sitemap routes based on properties of localisation (deferred from req.headers.host)

Then one would need access to the nuxt-i18n config in:

// nuxt.config.js

// Filter routes by language
{
  sitemap: {
    filter ({ routes, options }) {
      if (options.hostname === 'example.com') {
        return routes.filter(route => getNuxtI18nLocaleOf(route) === 'en')
      }
      return routes.filter(route => getNuxtI18nLocaleOf(route) === 'fr')
    }
  }
}

Just an example.

Could be written as:

// config/sitemap.js

// Filter routes by language
export default ({i18n}) => {
  
  function getNuxtI18nLocaleOf(route) {
      // Use i18n here to determine locale of route
      return ...
  }

  return {
    filter ({ routes, options }) {
      if (options.hostname === 'example.com') {
        return routes.filter(route => getNuxtI18nLocaleOf(route) === 'en')
      }
      return routes.filter(route => getNuxtI18nLocaleOf(route) === 'fr')
    }
  }
}
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.