-
Notifications
You must be signed in to change notification settings - Fork 12
[Vite] Migrate clustering-map from CRA to vite #65
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
Conversation
✅ Deploy Preview for sigma-org-chart canceled.
|
| <head> | ||
| <meta charset="utf-8" /> | ||
| <link rel="icon" href="%PUBLIC_URL%/favicon.ico" /> | ||
| <link rel="icon" href="/favicon.ico" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed due to vitejs/vite#6482
| @@ -0,0 +1,25 @@ | |||
| { | |||
| "compilerOptions": { | |||
| "target": "ES2020", | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These next two files are copied from actions-sample-plugin. Should we use ESNext instead of ES2020?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets just copy the actions sample plugin for now. I think a good task would be to refactor this whole repo to use shared configuration files so that we don't have to do this for each example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ES version shouldn't make a big difference here, but probably just safer to do ES2020 so the version is consistent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense, leaving as is for now
| "strict": true | ||
| }, | ||
| "include": ["vite.config.ts"] | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this file? I added since thats how the actions-sample-plugin has it set up, but not sure what exactly its purpose is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you don't include it, TS wont typecheck on it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, you meant the whole file. I think its fine to keep
| "strict": true | ||
| }, | ||
| "include": ["vite.config.ts"] | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, you meant the whole file. I think its fine to keep
Resolves SIG-41705
create-react-appis deprecated and we should start having customers use vite for setting up their plugins instead.actions-sample-pluginis already using vite. Migratingclustering-map.Mainly followed config from
actions-sample-plugin, vite is on major version 6 now though so not sure if i need to change anything?