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

Add TS types #3

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add TS types #3

wants to merge 3 commits into from

Conversation

vdegenne
Copy link

This commit adds TypeScript types to get rid of error such as :

Plugin typescript: @rollup/plugin-typescript TS2307: Cannot find module './test.css' or its corresponding type declarations.

@changesets/cli package was also added to the project to ease up external contribution.

@JulianCataldo
Copy link

I encountered the same situation and thought of monkey-patching the fact that TS still not support assert or with with CSS (JSON works though) microsoft/TypeScript#46689

Didn't tested your solution yet,
but assuming Vite is using ambient modules for foo.css?inline, etc., in a similar manner, we would lose an important DX feature in my eyes:
Cmd+Click to the CSS file.
With types, it will always point the same .d.ts, like with the newest Vite.

So it's a trade-off I guess ;) At least, if we keep the refactoring support, while shutting down TS complaint, it would still be a better situation.

Thx!

@vdegenne
Copy link
Author

vdegenne commented Dec 26, 2023

@JulianCataldo Nevertheless I think it's superior to Vite, the extra ?inline provokes VSCode to travel directly to the declaration file, whereas this solution will also bring the CSS file into light :
image
I wonder how long we still have to wait until TypeScript decides to support these imports natively, and if VSCode has an option to ignore specified declaration files.

@vdegenne
Copy link
Author

@justinfagnani Can you review this one? Thanks

@JulianCataldo
Copy link

JulianCataldo commented Jan 6, 2024

Need to point the the class prototype, though


Before:

Screenshot 2024-01-06 at 22 44 47

After:

env.d.ts:

declare module '*.css' {
	export default CSSStyleSheet.prototype;
}

declare module '*.scss' {
	export default CSSStyleSheet.prototype;
}
Screenshot 2024-01-06 at 22 45 57

Unrelated note: I do a transform / replace from .scss to .css before the TS is compiled to JS to the dist. But that's in my build sauce. I do that for keeping a handy CMD + Click to the source SCSS file while in dev.

Comment on lines +1 to +4
declare module '*.css' {
const stylesheet: CSSStyleSheet;
export default stylesheet;
}
Copy link
Author

Choose a reason for hiding this comment

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

@JulianCataldo Have you tried this syntax instead? which is the PR I proposed.
You can't directly return a type

Choose a reason for hiding this comment

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

Awesome! I was fiddling with const, var, default,… but couldn't recall the exact pattern, even though I'd seen it somewhere before. Thx

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

Successfully merging this pull request may close these issues.

2 participants