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

[1.x] Add opt-in TypeScript support #267

Merged
merged 8 commits into from Mar 18, 2023
Merged

[1.x] Add opt-in TypeScript support #267

merged 8 commits into from Mar 18, 2023

Conversation

jessarcher
Copy link
Member

@jessarcher jessarcher commented Mar 8, 2023

This PR introduces opt-in TypeScript support for the Vue and React stacks.

It can be installed by passing the --typescript flag to the installer. When no arguments are given to the installer, the user will be prompted if they select the Vue or React stack.

Some details on the implementation

I wanted to set a strong base standard for the community, so strict mode is enabled by default and the any type is not used in the scaffolded code. Of course, developers are free to relax this based on their needs. The rest of the TypeScript configuration is based on information from https://vitejs.dev/guide/features.html#typescript.

It's important to note that Vite only transpiles the TypeScript, but does not handle type checking. For that reason, tsc/vue-tsc is run as part of the build script.

The type noise in the components is pleasantly minimal while offering type safety and vastly improved IDE support.

The Vue components follow the conventions documented at https://vuejs.org/guide/typescript/composition-api.html.

The React components rely on existing React types so that components like <PrimaryButton> inherit the prop types for everything on the underlying <button>.

I have included the --typescript option in GitHub actions which runs the build script and validates the types, so future PRs cannot inadvertently ship type errors.

My only concern with this PR is that it introduces two new sets of stubs to maintain, which will be tedious when adding new features or redesigning. Ultimately I think it's worth it, though.

@heychazza
Copy link

Yes yes yes!! I've wanted to dive into TypeScript for a while but never knew where to begin. This will be a fantastic base (and learning curve).

@JKHarley
Copy link

JKHarley commented Mar 8, 2023

Great addition! Love that strict mode is enabled by default 👌 Thank you!!

@wlepinski
Copy link

Amazing 😍

Curious enough I was planning to work on something similar so I guess I need to find something else to be my first contribution here.

@emrancu
Copy link

emrancu commented Mar 8, 2023

It will be a great impactful addition ❤️.
Thank you!!

@tecbay
Copy link

tecbay commented Mar 9, 2023

Wiii make life easy! Thanks man

@retromack
Copy link

I tried to add default layout but getting 'page' is of type 'unknown' error
image

@jessarcher
Copy link
Member Author

I tried to add default layout but getting 'page' is of type 'unknown' error image

You can fix the unknown type by specifying the generic type on import.meta.glob, although I'm not sure what type to specify...

The value of page is an object with a key of default containing the component function:

image

I can't find an existing type for this object, but we can define it ourselves, which might look like this:

const page = await resolvePageComponent(`./Pages/${name}.tsx`, import.meta.glob<{ default: JSXElementConstructor<unknown> }>('./Pages/**/*.tsx'))

(I'm not a React expert so I'm not sure if that's actually correct)

The problem is that the layout key doesn't exist on any of React's types. I think it's an Inertia-specific convention leveraging JavaScript's ability to set arbitrary keys on a function.

So, we could also add that to our type definition, perhaps something like this:

const page = await resolvePageComponent(`./Pages/${name}.tsx`, import.meta.glob<{ default: JSXElementConstructor<unknown> & { layout?: JSXElementConstructor<undefined> } }>('./Pages/**/*.tsx'))

This seems to eliminate the errors, but I don't feel it brings any value for all the noise it introduces. Your TSX components could export anything and I don't think this would pick up on the mistake.

Personally, I'd just add a @ts-expect-error comment above and accept that this is an area where TypeScript doesn't bring value. Happy to be proven wrong though!

@Deniz073
Copy link

on tpyes/global.d.ts:
Cannot find module 'axios'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option?

on tsconfig.json line 5:
Argument for '--moduleResolution' option must be: 'node', 'classic', 'node16', 'nodenext'

on app.tsx:
image

on bootstrap.ts:
Cannot find module 'axios'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option?

on most of the .tsx files within resources/js:
Cannot find module '@inertiajs/react'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option?
Cannot find module '@headlessui/react'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option?
Cannot find module '@/types'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option?

Also when giving a ref to the TextInput component, the ref resolves to this in react devtools and not to the actual input element:
image

@retromack
Copy link

on tpyes/global.d.ts: Cannot find module 'axios'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option?

on tsconfig.json line 5: Argument for '--moduleResolution' option must be: 'node', 'classic', 'node16', 'nodenext'

on app.tsx: image

on bootstrap.ts: Cannot find module 'axios'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option?

on most of the .tsx files within resources/js: Cannot find module '@inertiajs/react'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option? Cannot find module '@headlessui/react'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option? Cannot find module '@/types'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option?

Also when giving a ref to the TextInput component, the ref resolves to this in react devtools and not to the actual input element: image

I think it is because you are using vscode typescript instead of the workspace one.

image

@Deniz073
Copy link

I think it is because you are using vscode typescript instead of the workspace one.

image

How did you get the dropdown in ur picture? my statusbar of vscode at the bottom does not show me the typescript version i need to click on as per the vscode docs https://code.visualstudio.com/docs/typescript/typescript-compiling#_using-the-workspace-version-of-typescript

@retromack
Copy link

retromack commented Mar 17, 2023

I think it is because you are using vscode typescript instead of the workspace one.
image

How did you get the dropdown in ur picture? my statusbar of vscode at the bottom does not show me the typescript version i need to click on as per the vscode docs https://code.visualstudio.com/docs/typescript/typescript-compiling#_using-the-workspace-version-of-typescript

From the Command Palette (Ctrl+Shift+P) just type Select Typescript Version

@Deniz073
Copy link

Deniz073 commented Mar 18, 2023

I think it is because you are using vscode typescript instead of the workspace one.
image

How did you get the dropdown in ur picture? my statusbar of vscode at the bottom does not show me the typescript version i need to click on as per the vscode docs https://code.visualstudio.com/docs/typescript/typescript-compiling#_using-the-workspace-version-of-typescript

From the Command Palette (Ctrl+Shift+P) just type Select Typescript Version

Thanks. This indeed fixed those errors for me.
However, the ref referencing the focus function instead of the input element problem still stands.

@jessarcher a possible solution could be defining useImperativeHandle like this:
useImperativeHandle(ref, () => ({ input: localRef.current, focus: () => { localRef.current?.focus(); } }));
Now, when ref is passed to the component from the parent component, it will resolve to an object with two properties: input which is the input element, and focus which is a function that focuses on the input element.

However when doing this and accessing the input element like nameRef.current?.input.value causes this typescript error:
Property 'input' does not exist on type 'HTMLInputElement'. Did you mean 'oninput'?ts(2551) lib.dom.d.ts(5957, 5): 'oninput' is declared here. Although the value of the input is accessed correctly.

@taylorotwell taylorotwell merged commit 1e82ae1 into 1.x Mar 18, 2023
21 checks passed
@taylorotwell taylorotwell deleted the typescript-support branch March 18, 2023 19:10
@drewmw5
Copy link
Contributor

drewmw5 commented Mar 20, 2023

Yay! Thank you Laravel team! <3

@rizkhal
Copy link

rizkhal commented Mar 21, 2023

Great works, Thanks!

@abderrahim-kharit
Copy link

i cannot get the command for creating a new project with typescript support.

like this: laravel new app --breeze

dosen't show the typescript options

thanks.

@faabiopontes
Copy link

Hi @abderrahim-kharit these are different repos, one is your usual Laravel installation, the other is for the Breeze package. Take a look at Laravel Breeze Installation, when issuing the php artisan breeze:install there will be the option to use TypeScript, or directly with the php artisan breeze:install --typescript

A step by step would be:

composer create-project laravel/laravel example-app
cd example-app
composer require laravel/breeze --dev
php artisan breeze:install --typescript

@adiramardiani
Copy link

Hi @jessarcher I'm sorry for mention

vue : 3.4.19
vue-tsc : 1.8.27
vite : 5.1.4
@inertiajs/vue3 : 1.0.14

I install fresh using latest breeze - laravel 11 dev, and I have some issue. $page.props is dont have auth user and ziggy
Using laravel 10 fresh install add breeze, the type is working well, I think this is only on laravel 11

Here my global.d.ts from breeze

import { PageProps as InertiaPageProps } from '@inertiajs/core';
import { AxiosInstance } from 'axios';
import { route as ziggyRoute } from 'ziggy-js';
import { PageProps as AppPageProps } from './';

declare global {
    interface Window {
        axios: AxiosInstance;
    }

    const route: typeof ziggyRoute;
}

declare module 'vue' {
    interface ComponentCustomProperties {
        route: typeof ziggyRoute;
    }
}

declare module '@inertiajs/core' {
    interface PageProps extends InertiaPageProps, AppPageProps {}
}

Here my index.d.ts from breeze

import { Config } from 'ziggy-js';

export interface User {
    id: number;
    name: string;
    email: string;
    email_verified_at: string;
}

export type PageProps<T extends Record<string, unknown> = Record<string, unknown>> = T & {
    auth: {
        user: User;
    };
    ziggy: Config & { location: string };
};

I think this is just my IDE problem, but here my type check
image

Or I have some error/missmatch config ?

@adiramardiani
Copy link

I think this issue will solve on inertia inertiajs/inertia#1794
once this merge, I hope the experimental typescript will working well from 'red line' @jessarcher :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet