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

Rewrite internals (road to v1 ✨) #44

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

Conversation

Temzasse
Copy link
Owner

Road to v1

Things done:

  • Improve memoization for style calculation
  • Resolve media and util styles correctly
    • Allow usage of utils in other utils
    • Allow having media styles in utils, variants, and compound variants
  • Add tests for internal helper functions
  • Add more rendering based tests for various cases

Todo:

  • Add even more tests 😄
  • Test on simulator / real device / web browser
  • Make the example app be more like a real-world app with proper examples of the different features

This rewrite should address these issues/PRs:

Since @tkow you've been so active in this project lately I would love to hear your thoughts about this rewrite 😊 Have I missed something important or is there something that we should add to the lib before we publish the v1.0.0 version?

@Temzasse Temzasse self-assigned this Dec 28, 2022
@tkow
Copy link
Contributor

tkow commented Dec 29, 2022

@Temzasse
Thanks for many fixes. These are more concise to understand intention of codes than mine.
I agree with you to improve tests, too. I think the road map is enough to fix lack of features by my modifications.

BTW, though I think v1.0.0 doesn't need it, how do you think about rewriting this library using Typescript? I feel it easier to modify than js codes.
I recently like to create typescript library with swc or esbuild which accelerates jest running and the build output can be commonjs and es format.
If you feel like it, I may help introducing it after v1.0.0.

@Temzasse
Copy link
Owner Author

@tkow good point about rewriting the internals in TypeScript 👍 I agree that it would improve the development experience if we used TS instead of JS.

I initially tried to write the library purely with TS but it turned out to be very difficult 😅 I think there is a reason why the original Stitches lib kept the typings separate from the library internals since the types are so complex. I still don't fully understand all the TS black magic that is happening in the type defs that I ported over 😆

If we can keep using the separate type defs while converting the internals to TS I think that would be great. However I'm not sure if that will cause problems when we build and package the library for npm 🤔

@tkow
Copy link
Contributor

tkow commented Dec 29, 2022

@Temzasse

Thank you for advices.
In my experience maybe I can reuse ambient files with no modifications and, write source files using typescripts.
I challenge to replace js to ts when I have time!

Ah... . I noticed border style examples I added are not suitable for test native app.
Sorry for that, let me think the alternatives.

@tkow
Copy link
Contributor

tkow commented Dec 31, 2022

@Temzasse
Though it's not rerated to this roadmap, why don't you deploy this package from Github Actions ?

I made it before with using changesets in a repository. The official changesets workflow is available and relatively simple to configure (But, this action have a pitfall to commit .npmrc including secret npm token (it has risk to leak your npm token) if you don't avoid it by .gitignore or committing your own .npmrc file).

Even if you don't familiar with changesets, you can get started by only a few things you should know .

  1. Write a change log created by npx changesets command and commit the file your working branch.
  2. If the change log pushed branches which triggers changesets/actions above, the action generate automatically branch and PR from it to publish npm.
  3. If the changesets's file is merged by the generated PR, the action publishes npm follows configurations when the PR closes without errors.

If you're interested CI deploy management. Please feel free to ask me about it.

@Temzasse
Copy link
Owner Author

Temzasse commented Jan 2, 2023

@tkow
The current release flow has been working pretty well for me so I haven't had the need for automating things 🙂

I'm using np for publishing the package to npm. It automatically creates Github releases with commit messages as the changelog. Also when I'm publishing the package I need to enter a 2fa code in the terminal - does the Github Actions publishing workflow handle that?

@tkow
Copy link
Contributor

tkow commented Jan 3, 2023

does the Github Actions publishing workflow handle that?

No, it doesn't. It works with bypassing 2-FA code tokens. Automation work flow trades off security though latest npm token format can be configured about package scope and expiration date.

The current release flow has been working pretty well for me so I haven't had the need for automating things

Sure. If it looks preferable to use 2-fa, please forget my suggestion.

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