Skip to content

Use .ts files for vite config for type safety #2912

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.

Sign up for GitHub

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

Draft
wants to merge 5 commits into
base: livekit
Choose a base branch
from
Draft

Use .ts files for vite config for type safety #2912

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
99c5a1a
Use .ts files for vite config for type safety
hughns Dec 18, 2024
7783c72
Only set sentry release name
hughns Dec 18, 2024
f09aad2
Merge branch 'livekit' into hughns/type-check-built-config
hughns Dec 18, 2024
4be011f
Merge upstream
hughns Jan 6, 2025
6c16c7a
Revert skipLibCheck
hughns Jan 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion i18next-parser.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
export default {
/*
Copyright 2024 New Vector Ltd.

SPDX-License-Identifier: AGPL-3.0-only
Please see LICENSE in the repository root for full details.
*/

import type { UserConfig } from "i18next-parser";

const config: UserConfig = {
keySeparator: ".",
namespaceSeparator: false,
contextSeparator: "|",
Expand Down Expand Up @@ -26,3 +35,5 @@ export default {
input: ["src/**/*.{ts,tsx}"],
sort: true,
};

export default config;
4 changes: 3 additions & 1 deletion tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
// Workaround for https://github.com/microsoft/TypeScript/issues/55132
"useDefineForClassFields": false,
"allowImportingTsExtensions": true,
"skipLibCheck": true,
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be a step back in how strict we do our typing.
Are we sure we want this? Is there no way to fix the reason we add this with yarns resolutions?
Can it get a comment why we need this now?

Copy link
Member Author

Choose a reason for hiding this comment

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

I concur that this is a backwards step. Unfortunately it's not just the case of some resolutions.

The main culprit appears to be @codecov/vite-plugin.

I will make this draft and lift the one material change over to a simpler PR.

"paths": {
// These imports within @livekit/components-core and
// @livekit/components-react are broken under the "bundler" module
Expand Down Expand Up @@ -50,6 +51,7 @@
"include": [
"./node_modules/matrix-js-sdk/src/@types/*.d.ts",
"./src/**/*.ts",
"./src/**/*.tsx"
"./src/**/*.tsx",
"*.ts"
]
}
10 changes: 6 additions & 4 deletions vite.config.js → vite.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ export default defineConfig(({ mode }) => {
ref: true,
},
}),
htmlTemplate.default({
// types workaround for https://github.com/IndexXuan/vite-plugin-html-template/issues/37
(htmlTemplate as unknown as { default: typeof htmlTemplate }).default({
data: {
title: env.VITE_PRODUCT_NAME || "Element Call",
},
Expand All @@ -53,8 +54,9 @@ export default defineConfig(({ mode }) => {
) {
plugins.push(
sentryVitePlugin({
include: "./dist",
release: process.env.VITE_APP_VERSION,
release: {
name: process.env.VITE_APP_VERSION,
},
}),
);
}
Expand All @@ -67,7 +69,7 @@ export default defineConfig(({ mode }) => {
sourcemap: true,
rollupOptions: {
output: {
assetFileNames: ({ originalFileNames }) => {
assetFileNames: ({ originalFileNames }): string => {
if (originalFileNames) {
for (const name of originalFileNames) {
// Custom asset name for locales to include the locale code in the filename
Expand Down
10 changes: 9 additions & 1 deletion vitest.config.js → vitest.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
/*
Copyright 2024 New Vector Ltd.

SPDX-License-Identifier: AGPL-3.0-only
Please see LICENSE in the repository root for full details.
*/

import { defineConfig, mergeConfig } from "vitest/config";
import viteConfig from "./vite.config.js";

import viteConfig from "./vite.config";

export default defineConfig((configEnv) =>
mergeConfig(
Expand Down
Loading