Skip to content

feat(create-vite): add preact path alias in preact-ts template#13499

Closed
AJamesPhillips wants to merge 1 commit intovitejs:mainfrom
AJamesPhillips:main
Closed

feat(create-vite): add preact path alias in preact-ts template#13499
AJamesPhillips wants to merge 1 commit intovitejs:mainfrom
AJamesPhillips:main

Conversation

@AJamesPhillips
Copy link
Copy Markdown

@AJamesPhillips AJamesPhillips commented Jun 13, 2023

Description

When adding react-redux package to a vite preact-ts project created from the template I got stuck on a cryptic error message: https://stackoverflow.com/questions/76440285/error-property-children-is-missing-in-type-vnodeany-but-required-in-type?noredirect=1#comment134806152_76440285

Type 'Element' is not assignable to type 'ReactNode'.
  Property 'children' is missing in type 'VNode<any>' but required in type 'ReactPortal'.ts(2322)
index.d.ts(166, 9): 'children' is declared here.
Provider.d.ts(19, 5): The expected type comes from property 'children' which is declared here on type 'IntrinsicAttributes & ProviderProps<AnyAction, unknown>'
(property) JSXInternal.IntrinsicElements.div: JSXInternal.HTMLAttributes<HTMLDivElement>

This was resolved by following the typescript preact/compat config guidelines here: https://preactjs.com/guide/v10/typescript/#typescript-preactcompat-configuration

This PR adds the preact/compat config to the preact-ts template for future users of create-vite.

Additional context

n/a


What is the purpose of this pull request?

  • Bug fix
  • New Feature (but perhaps treat as other because its improved default behaviour?)
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the PR Title Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@bolt-new-by-stackblitz
Copy link
Copy Markdown

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

"lib": ["ES2020", "DOM", "DOM.Iterable"],
"skipLibCheck": true,

"baseUrl": "./",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this baseUrl needed? The downside with this is that TypeScript would autocomplete imports as "src/components/..." (note leading src/), which wouldn't work in Vite ootb.

Copy link
Copy Markdown
Author

@AJamesPhillips AJamesPhillips Jun 30, 2023

Choose a reason for hiding this comment

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

Ah interesting, I didn't know that thanks. I'm using the baseUrl in this vitejs project: github.com/AJamesPhillips/conversation-helper and when I use VisualStudio code to add a missing import it adds as a relative path like "./components/LogList". How do you recreate the autocomplete imports as "src/..." is this a setting I have set that I'm now unaware of perhaps?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It usually happens when you have a very long path of ../../../../../../ and TS intelligently picks the short src/components/.. path instead. But anyways, setting this baseUrl also let TS resolves correctly without warnings, so a user who manually type that (for any reason) would falsely think it works too.

Copy link
Copy Markdown
Author

@AJamesPhillips AJamesPhillips Jun 30, 2023

Choose a reason for hiding this comment

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

Ok. Makes sense. I just wish there was a more descriptive error message to point you towards adding "baseUrl" and "paths" to tsconfig.json

Sounds like you don't want this contribution so I'll close this PR. Thanks for your time and help understanding this better 🙂

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I do want the paths contribution! Just not the baseUrl part. Is the baseUrl required for paths to work?

Copy link
Copy Markdown
Author

@AJamesPhillips AJamesPhillips Jun 30, 2023

Choose a reason for hiding this comment

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

Ah ok. I misunderstood. In my own project it seems I can remove "baseUrl" and the following still work without error: imports, type checking, and vite dev/build. So I will hesitantly say yes but I don't really know why preact would recommend adding baseUrl if it wasn't needed.

@bluwy bluwy added the p2-nice-to-have Not breaking anything but nice to have (priority) label Jun 20, 2023
@AJamesPhillips
Copy link
Copy Markdown
Author

Will reopen to bump this comment: #13499 (comment) but happy to close as I'm not confident about this change.

@sapphi-red
Copy link
Copy Markdown
Member

Thanks for the PR! I'll close this one as the main part seems to be merged by #14262 🙏

@sapphi-red sapphi-red closed this Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p2-nice-to-have Not breaking anything but nice to have (priority)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants