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

fix(nuxt): make it possible to overwrite default values for css option (#4214) #4214

Merged
merged 1 commit into from
Apr 8, 2024

Conversation

cybearz
Copy link
Contributor

@cybearz cybearz commented Apr 6, 2024

Description

Vuestic's option "css" accepts an array of css modules as an input.
The default value of that option is ['smart-helpers', 'typography'].
The problem is that nuxt won't override the default array with an array passed by user (it's a known issue). Instaed two arrays will be concatenated.

For example, I want to use only a typography css module, so I set the css option to ['typography'].
image
In that case I expect the css option to be ['typography'] according to documentations.
Instead i have this:
image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any feature but make things better)

Vuestic's option "css" accepts an array of  css modules as an input. The default value of that option is ['smart-helpers', 'typography'].
The problem is that nuxt won't override the default array with an array passed by user (it's a known issue). Instaed two arrays will be concatenated.
For example, I want to use only a typography css module, so I set the css option to ['typography']. In that case useVuesticCSS method wil receive ['smart-helpers', 'typography', 'typography'] and not just ['typography']
@cybearz cybearz changed the title Workaround for using arrays as default options in nuxt module closes #4213 fix:Workaround for using arrays as default options in nuxt module closes #4213 Apr 7, 2024
@m0ksem m0ksem self-requested a review April 8, 2024 02:41
@m0ksem
Copy link
Contributor

m0ksem commented Apr 8, 2024

I agree this is counterintuitive, but I'm wondering if we should obey the rules defined by nuxt team. (see https://nuxt.com/docs/guide/directory-structure/app-config#merging-strategy)

Someone might expect arrays to be merged (I don't really believe in this, though). In case you want to completely override this option, you can use a function, as describe in link above.

@m0ksem
Copy link
Contributor

m0ksem commented Apr 8, 2024

I haven't found any modules that make something like this. (https://nuxt.com/modules/).

What I can propose is to move to object format:

css: {
  typography: false,
  reset: false,
}

@m0ksem
Copy link
Contributor

m0ksem commented Apr 8, 2024

Another option is making typography a separate option.

typography: {
   font: 'Inter',
   helpers: false, // disable typography helpers like .va-h1, etc.
}

I think we can totally remove grid, smart-helpers and reset, because we aim for tailwind to take responsibility for that.

@m0ksem
Copy link
Contributor

m0ksem commented Apr 8, 2024

Okay. Looks like merging function don't work.

Both adds 'smart-heleprs' anyway:

  vuestic: () => ({
    config: {
      // Config here
    },
    css: ['typography']
  }),
  vuestic: {
    config: {
      // Config here
    },
    css: () => ['typography']
  },

Copy link
Contributor

@m0ksem m0ksem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@m0ksem m0ksem merged commit 2ff3515 into epicmaxco:develop Apr 8, 2024
@m0ksem m0ksem changed the title fix:Workaround for using arrays as default options in nuxt module closes #4213 fix(nuxt): make it possible to overwrite default values for css option (#4214) Apr 8, 2024
@cybearz
Copy link
Contributor Author

cybearz commented Apr 8, 2024

Thanks!

Or maybe it would be better to set "css: true" which will register all CSS by default? In that case if the user passes an array with the css modules he needs, there will be no errors with merging (since the default value is not an array or an object).

@m0ksem
Copy link
Contributor

m0ksem commented Apr 8, 2024

I think the current solution you proposed is more than enough. We'll need to refine how CSS features work. We have two builds for typography: global and just css classes. As for now, there is no way to control build in nuxt.

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.

None yet

2 participants