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 support, cleans up modules #335

Merged
merged 8 commits into from Jan 24, 2023
Merged

add TS support, cleans up modules #335

merged 8 commits into from Jan 24, 2023

Conversation

argyleink
Copy link
Owner

fixes #333

microbundle wasnt building friendly files, making them hard to infer types from and impossible for authors to read. this drastically simplifies the build output for js modules while also adding a .ts file and .d.ts typings file.

preview of open-props.ts:

export default {
  "--animation-fade-in": "fade-in .5s var(--ease-3)",
  "--animation-fade-in-@": "\n@keyframes fade-in {\n  to { opacity: 1 }\n}",
  "--animation-fade-in-bloom": "fade-in-bloom 2s var(--ease-3)",
  ...
}

preview of open-props.d.ts:

declare const OpenProps: {
  "--animation-fade-in": "string";
  "--animation-fade-in-@": "string";
  "--animation-fade-in-bloom": "string";
  "--animation-fade-in-bloom-@": "string";
  ...
}

thoughts @mayank99 or @hchiam?

no new deps, no invasive or fancy ts; just an easier to infer dist file for TS to grab

@stackblitz
Copy link

stackblitz bot commented Jan 20, 2023

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

@github-actions
Copy link

github-actions bot commented Jan 20, 2023

Visit the preview URL for this PR (updated for commit 6505111):

https://open-props--pr335-ts-autocomplete-j2g6ul5x.web.app

(expires Tue, 31 Jan 2023 01:41:04 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 32524ac481f54edda55dc959fa4614cf1acc8c11

Copy link
Contributor

@mayank99 mayank99 left a comment

Choose a reason for hiding this comment

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

so now there will be typings for the default export from root, but not for any additional paths. is the goal to stop here or to allow a separate PR to generate those additional DTS files?

build/props.js Outdated Show resolved Hide resolved
Copy link
Contributor

@hchiam hchiam left a comment

Choose a reason for hiding this comment

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

i've got nothing to add beyond @mayank99's comment #335 (comment)
LGTM! 👍

@argyleink
Copy link
Owner Author

but not for any additional paths. is the goal to stop here or to allow a separate PR to generate those additional DTS files?

i was going to stop here, but it's not much more effort to produce individual .d.ts files now the system is laid out. it'd be good to have parity in the import syntax, just so the docs can stay clean and dont diverge 👍🏻 aka, assume i'll add to this PR

@argyleink
Copy link
Owner Author

Screenshot 2023-01-23 at 3 42 43 PM

@argyleink
Copy link
Owner Author

alright y'all, i'll merge this tomorrow if we're feelin good about it. thanks for everyones help and effort!

@argyleink
Copy link
Owner Author

oh! or i need to figure out why the test failed first 🙂

test/basic.test.cjs Outdated Show resolved Hide resolved
build/props.js Outdated Show resolved Hide resolved
test/basic.test.cjs Outdated Show resolved Hide resolved
Co-authored-by: Mayank <9084735+mayank99@users.noreply.github.com>
Co-authored-by: Howard Chiam <hchiam@users.noreply.github.com>
build/props.js Outdated Show resolved Hide resolved
test/basic.test.cjs Outdated Show resolved Hide resolved
build/props.js Show resolved Hide resolved
@argyleink
Copy link
Owner Author

super glad y'all're makin suggestions so we can collab on this PR 🙂
i'm also at home with kids hehe, they came home right when i was accepting the first round of suggestions

@argyleink argyleink merged commit 109ad16 into main Jan 24, 2023
@argyleink argyleink deleted the ts-autocomplete branch January 24, 2023 02:42
@mayank99
Copy link
Contributor

yo, you said tomorrow. i was just in the process of testing this with npm pack + local apps lol

no worries, i'll test with actual 1.5.4

@argyleink
Copy link
Owner Author

ah, sorry! i did. it just seemed ready and done.

i updated https://stackblitz.com/edit/react-vanillaextract-openprops to latest and typeahead was there. how'd it go for you? all good? new gitignore got em all?

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.

type definitions?
3 participants