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

feat: layer meta and source options #62

Merged
merged 8 commits into from
Mar 13, 2023
Merged

feat: layer meta and source options #62

merged 8 commits into from
Mar 13, 2023

Conversation

pi0
Copy link
Member

@pi0 pi0 commented Mar 10, 2023

A New $meta config is supported for each configuration. This is mainly useful in conjunction with extended layers which can provide their metadata (such as name, version, etc).

Secondly, when extending a layer, users can provide custom source options { source: 'extend-source', options: {} } or source: ['extend-source, { ...options }]. This is useful for defining options when extending a layer. Passing options such as cloning tokens.

It is possible to override layer config and meta also using opts.overrides and opts.meta but it is probably for specific cases, such as multi-app support when we need to override a layer. we might not advocate this in general to users but gives an escape hatch to configure a layer meta itself by the consumer.

@pi0 pi0 requested review from atinux and danielroe March 10, 2023 15:30
@pi0
Copy link
Member Author

pi0 commented Mar 10, 2023

Side note: For framework consumers, we can use this meta to show beautiful banners (ie: nuxi cli)

@codecov
Copy link

codecov bot commented Mar 10, 2023

Codecov Report

Merging #62 (9f71e9f) into main (071180f) will increase coverage by 0.06%.
The diff coverage is 85.36%.

@@            Coverage Diff             @@
##             main      #62      +/-   ##
==========================================
+ Coverage   87.20%   87.27%   +0.06%     
==========================================
  Files           3        3              
  Lines         469      503      +34     
  Branches       60       65       +5     
==========================================
+ Hits          409      439      +30     
- Misses         60       64       +4     
Impacted Files Coverage Δ
src/loader.ts 91.24% <85.36%> (-0.32%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@atinux
Copy link
Member

atinux commented Mar 10, 2023

Actually I would prefer to only support a $meta property in the source itself and keep the extends as an array of string.

Each source should specify the $meta like a Nuxt module is specifying the metadata.

I don't see an usage of overwriting the meta honestly.

The only usage I could see is to overwrite the layer config, but IMO it can be done by adding another layer after it:

{
  extends: [
    './base-layer.ts',
    './my-overwrite.ts'
  ]
}

@pi0
Copy link
Member Author

pi0 commented Mar 10, 2023

Sources define $meta :)

meta overwrite support is particularly useful for multi-apps when we want to configure an app layer or toggle its features. (config for a specifc layer only not the merged result). Also extending some layers, needs more than just a source string. Like a token for cloning them (and we don't want a merged token! Each layer might need it's token)

For nuxt modules, it is comparable to module defaults and user options for module (meta, despite it's name is both static meta and layer-source configuration)

@pi0 pi0 changed the title feat: extended layer meta feat: layer meta and source options Mar 10, 2023
src/loader.ts Show resolved Hide resolved
src/loader.ts Outdated Show resolved Hide resolved
src/loader.ts Show resolved Hide resolved
src/loader.ts Show resolved Hide resolved
@atinux
Copy link
Member

atinux commented Mar 11, 2023

I do believe we need to split this PR into two:

  • layer metadata: exposing $meta in the config part, very useful for Nuxt to display what layer names are extended
  • source options: useful to specific a way to fetch a layer that need some options (ex: studio:docus that could need a token, even though I believe this could be fixed by supporting more giget providers)

@atinux
Copy link
Member

atinux commented Mar 11, 2023

Regarding multi apps, I would like to better understand the idea.

meta overwrite support is particularly useful for multi-apps when we want to configure an app layer or toggle its features. (config for a specifc layer only not the merged result)

I would like to see an example, because the meta data !== that overwriting the config options. Except if you are thinking of a namespace. But IMO it is somehow at higher level than layers (kind of apps key).

Or we should define what metadata have a special purpose into Nuxt.

Always best to think is to imagine an example for Nuxt:

export default defineNuxtConfig({
  extends: [
    './ui', // global layer for all apps
    ['./admin', { baseURL: '/admin/' }] // only for /admin/ ?
  ]
})

It is still not clear how to bring micro-frontend to Nuxt right now, would love to discuss with an RFC first because it is a big subject.

I do believe the $meta in layer is a nice thing that could already improve the DX when using extends in Nuxt for both logging and devtools so far.

Source options, I would like to see when it is required or how we can improve.

Multi apps: can we write a readme on how to create them in Nuxt?

@pi0
Copy link
Member Author

pi0 commented Mar 13, 2023

Thanks for the reviews @atinux.

So far, what C12 will provide are very basic and non-opinionated building blocks for conf layer meta and source options.

    1. Static $meta per config layers defining metadata such as name, version, url, ...
    1. User-defined layer options for defining cloning options (such as a token) and possibly overriding layer behavior

Re Nuxt, we definitely need to explore how we imagine Multi-App support and also use extends meta. Allowed types and behavior is finally defined at Framework level. But all functionalities in c12 is essentially the first step to explore those ideas. I am happy to deprecate some options from c12 if finally there was no usage or introducing (unpreventable) anti-patterns.

Let's continue discussion about Nuxt together with @danielroe in next steps :)

src/loader.ts Outdated Show resolved Hide resolved
@pi0 pi0 merged commit ec03711 into main Mar 13, 2023
@pi0 pi0 deleted the feat/layer-meta branch March 13, 2023 11:43
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.

2 participants