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

clearScreen option can't be overwritten via plugin #10780

Closed
7 tasks done
qodesmith opened this issue Nov 4, 2022 · 3 comments · Fixed by #10787
Closed
7 tasks done

clearScreen option can't be overwritten via plugin #10780

qodesmith opened this issue Nov 4, 2022 · 3 comments · Fixed by #10787
Labels
contribution welcome p3-minor-bug An edge case that only affects very specific usage (priority)

Comments

@qodesmith
Copy link
Contributor

qodesmith commented Nov 4, 2022

Describe the bug

If I write a plugin that tries to set clearScreen: false in the config, it has no effect.

I know I can simply pass the option directly to defineConfig, but I'm reporting this because it's part of a plugin I'm writing that does other stuff but wants to ensure the screen is not cleared.

Reproduction

https://github.com/qodesmith/vite-keep-screen

Steps to reproduce

Neither keepScreen1 or keepScreen2 plugins will work. This is to showcase the 2 ways the documentation says you can update a config (via deep merge or directly editing the config object).

// vite.config.ts
import {defineConfig, PluginOption} from 'vite'
import react from '@vitejs/plugin-react'

// https://vitejs.dev/config/
export default defineConfig({
  plugins: [react(), keepScreen1()],
})

function keepScreen1(): PluginOption {
  return {
    name: 'vite-plugin-keep-screen1',
    config() {
      return {clearScreen: false}
    },
  }
}

function keepScreen2(): PluginOption {
  return {
    name: 'vite-plugin-keep-screen2',
    config(config) {
      config.clearScreen = false
    },
  }
}

System Info

System:
    OS: macOS 12.6
    CPU: (8) arm64 Apple M1 Pro
    Memory: 239.61 MB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 18.6.0 - ~/.nvm/versions/node/v18.6.0/bin/node
    npm: 8.13.2 - ~/.nvm/versions/node/v18.6.0/bin/npm
  Browsers:
    Brave Browser: 107.1.45.116
    Safari: 16.0
  npmPackages:
    @vitejs/plugin-react: ^2.2.0 => 2.2.0 
    vite: ^3.2.0 => 3.2.2

Used Package Manager

npm

Logs

No response

Validations

@bluwy
Copy link
Member

bluwy commented Nov 4, 2022

Looks like we create the logger with the clearScreen option too early:

const logger = createLogger(config.logLevel, {
allowClearScreen: config.clearScreen,
customLogger: config.customLogger
})

Feel free to send a PR to move this down!

@bluwy bluwy added contribution welcome p3-minor-bug An edge case that only affects very specific usage (priority) and removed pending triage labels Nov 4, 2022
@github-actions
Copy link

github-actions bot commented Nov 4, 2022

Hello @qodesmith. We like your proposal/feedback and would appreciate a contribution via a Pull Request by you or another community member. We thank you in advance for your contribution and are looking forward to reviewing it!

@qodesmith
Copy link
Contributor Author

PR is up!

@github-actions github-actions bot locked and limited conversation to collaborators Nov 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
contribution welcome p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants