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

docs: Fix typescript errors in search components #2500

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dwgray
Copy link
Collaborator

@dwgray dwgray commented Jan 9, 2025

Describe the PR

This is the last set of typescript errors in docs. Once this and the other PRs in flight are merged, I'll push a change to add type-check to the main build, which should prevent new errors from being checked in.

I went down a bunch of paths to try to suppress these errors, since they aren't ours. However, it appears that the typescript team is opposed to suppressing errors in the dependency tree (you can exclude top-level files, but not things that are included by a file that you're checking. See microsoft/TypeScript#40426 for info.

I've opened a feature request on VitePress to see if they have other ideas on how to include their search control in a custom theme vuejs/vitepress#4476 - but I'd like to get type checking turned on sooner then they're likely to react.

The other possibility is to rework our theme to be an extended default theme so that we can use their Layout component. But that seems like it would be a mess, and one of the nice things about our doc site as it stands is that we're using BSVN for most of the styling which I'm not sure we could do if we went down this route.

Small replication

pnpm type-check

PR checklist

What kind of change does this PR introduce? (check at least one)

  • Bugfix 🐛 - fix(...)
  • Feature - feat(...)
  • ARIA accessibility - fix(...)
  • Documentation update - docs(...)
  • Other (please describe)

The PR fulfills these requirements:

  • Pull request title and all commits follow the Conventional Commits convention or has an override in this pull request body This is very important, as the CHANGELOG is generated from these messages, and determines the next version type. Pull requests that do not follow conventional commits or do not have an override will be denied

Copy link

stackblitz bot commented Jan 9, 2025

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link

pkg-pr-new bot commented Jan 9, 2025

Open in Stackblitz

npm i https://pkg.pr.new/bootstrap-vue-next/bootstrap-vue-next/@bootstrap-vue-next/nuxt@2500
npm i https://pkg.pr.new/bootstrap-vue-next/bootstrap-vue-next@2500

commit: 3ffa239

@dwgray
Copy link
Collaborator Author

dwgray commented Jan 9, 2025

@VividLemon It looks like they fixed this (amazing turnaround) - between now and the next release, do you have a preference between letting this go through or putting up a new PR against the VP nightly build? I think there are arguments either way - this is more targeted (other issues may creap in with the nightly build, nightly builds tend to be less tested, etc), on the other hand if we point this to the nightly build, this will just work when we move to the next release...

@dwgray
Copy link
Collaborator Author

dwgray commented Jan 9, 2025

@VividLemon It looks like they fixed this (amazing turnaround) - between now and the next release, do you have a preference between letting this go through or putting up a new PR against the VP nightly build?

@VividLemon On further though, I think this method is clearly better. I did a bit of poking around to check for unexpected regressions and things look fine. I'm going to move #2501 to draft in case you think otherwise, will close one or the other of these once the other one goes through.

@dwgray dwgray marked this pull request as draft January 9, 2025 19:03
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.

1 participant