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: improve plugin params typing #2758

Closed
wants to merge 2 commits into from
Closed

Conversation

fnlctrl
Copy link
Member

@fnlctrl fnlctrl commented Dec 8, 2020

support plugin params type inference

Close #2589, Closes #2588

@LinusBorg
Copy link
Member

@fnlctrl Does this PR have any advantage over the earlier #2589 ?

Copy link
Member

@pikax pikax left a comment

Choose a reason for hiding this comment

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

A few more tests

@@ -0,0 +1,31 @@
import { createApp, App } from './index'
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import { createApp, App } from './index'
import { createApp, App, Plugin } from './index'
const app = createApp({})

app.use(PluginClass, { foo: [123] })
// no error
app.use(PluginClass, { foo: ['asdf'] })
})
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
})
})
describe('plugin function', () => {
const plugin = (app: App, config: { num: number; url: string }) => {}
// @ts-expect-error no arguments
app.use(plugin)
// @ts-expect-error invalid arguments
app.use(plugin, {})
app.use(plugin, {
// @ts-expect-error invalid type
num: '',
url: ''
})
app.use(plugin, {
num: 1,
url: ''
})
app.use(
plugin,
{
num: 1,
url: ''
},
// @ts-expect-error too many arguments
true
)
})
describe('no argument', () => {
const plugin = (app: App) => {}
app.use(plugin)
// @ts-expect-error too many arguments
app.use(plugin, {})
})
describe('using plugin type', () => {
const plugin = ({} as unknown) as Plugin
app.use(plugin)
app.use(plugin, true)
app.use(plugin, {})
app.use(plugin, 1, 'string', {}, () => {})
})

import { createApp, App } from './index'

describe('plugin params inference', () => {
const app = createApp({})
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const app = createApp({})

@HcySunYang HcySunYang added 🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. scope: types labels Mar 31, 2021
@HcySunYang HcySunYang requested a review from pikax March 31, 2021 04:39
@tony19
Copy link
Contributor

tony19 commented Jun 17, 2021

This PR no longer compiles as of de954f4: demo

@andria-dev
Copy link

Is this PR outdated? If so, it might make sense to close this if no one is working on it. If not, is there a timeline for when this fix can be merged? From my perspective, the changes to packages/runtime-core/src/apiCreateApp.ts look solid.

@antfu
Copy link
Member

antfu commented Oct 3, 2022

Thanks for the PR. Close in favor of #3969

@antfu antfu closed this Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. scope: types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No typings for plugin options
7 participants