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

@lexical/playground: Moved Vite config to TS and removed code duplication #5744

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

StyleT
Copy link
Contributor

@StyleT StyleT commented Mar 20, 2024

Here we extract module resolution config to viteModuleResolution.ts and move both Vite configs from .js to .ts

Copy link

vercel bot commented Mar 20, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 20, 2024 6:25pm
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 20, 2024 6:25pm

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 20, 2024
@StyleT StyleT changed the title chore(@lexical/playground): Moved Vite config to TS and removed code duplication @lexical/playground: Moved Vite config to TS and removed code duplication Mar 20, 2024
Copy link
Collaborator

@ivailop7 ivailop7 left a comment

Choose a reason for hiding this comment

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

LGTM, I do think we can have the vite config as .mts (https://vitejs.dev/guide/troubleshooting#config) if we want to move more towards ESM practices.

@ivailop7
Copy link
Collaborator

Bumping the vite to v5 in package.json has been in my do list for a while. In case you want to give it a go.

Copy link
Collaborator

@ivailop7 ivailop7 left a comment

Choose a reason for hiding this comment

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

@StyleT I think this is good to be merged

@StyleT StyleT merged commit 4c37c10 into facebook:main Mar 28, 2024
45 checks passed
replacements: [
{
from: /__DEV__/g,
to: 'true',

Choose a reason for hiding this comment

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

should this be false?

@etrepum
Copy link
Collaborator

etrepum commented Mar 29, 2024

This reverted the /esm/ example in the playground from #5618, I'll create another PR to restore it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants