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

feat(babel-plugin): initial version #16

Merged
merged 12 commits into from
Apr 18, 2020
Merged

Conversation

tatchi
Copy link
Contributor

@tatchi tatchi commented Apr 9, 2020

My attempt to implement #10. Don't be afraid by all the files, lots of them are actually tests 😅

Don't hesitate if you have any comments, that's the first time I create a babel plugin so there are chances things are not implemented in an idiomatic way.

Let me know also from the tests if something is missing/not working properly. There's still at least one thing that I'm aware of which is not working: if one has already imported the hook using an alias

import { useStyling as useCustomName } from `glaze`

The plugin will detect that the hook is already imported but won't use the alias to call the hook:

const sx = useStyling(); // should be useCustomName()

There's a test importAlias to should highlight that.

.eslintignore Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Apr 9, 2020

This pull request introduces 50 alerts when merging 9a594f9 into aaf63d8 - view on LGTM.com

new alerts:

  • 48 for Unused variable, import, function or class
  • 2 for Superfluous trailing arguments

@tatchi
Copy link
Contributor Author

tatchi commented Apr 9, 2020

I fixed some of the eslint errors. Normally the remaining ones are due to the test files.

Do you know how could I disable the check for these files?

@lgtm-com
Copy link

lgtm-com bot commented Apr 9, 2020

This pull request introduces 50 alerts when merging fe61505 into 341f1b2 - view on LGTM.com

new alerts:

  • 48 for Unused variable, import, function or class
  • 2 for Superfluous trailing arguments

@kripod
Copy link
Owner

kripod commented Apr 9, 2020

Thank you very much for all the efforts made towards making this feature available! I'll try my best to review the code as soon as possible, trying to fix some issues while learning Babel myself 😄

@lgtm-com
Copy link

lgtm-com bot commented Apr 9, 2020

This pull request introduces 50 alerts when merging b012344 into 08cc3fd - view on LGTM.com

new alerts:

  • 48 for Unused variable, import, function or class
  • 2 for Superfluous trailing arguments

@kripod
Copy link
Owner

kripod commented Apr 12, 2020

Currently, I'm working on multi-page docs before releasing v0.5. After that, I'll be happy to inspect and review these wonderful contributions. In the meantime, I thought about adding proper autocomplete for TypeScript and had the idea that the sx prop could be allowed by:

/* 'treat.d.ts', placed in projects which depend on glaze */

import 'glaze/sx-prop'; // A '.d.ts' file in the glaze package root


// Just the usual from below, as seen in 'packages/example-gatsby':

import { tokens } from './theme.treat';

declare module 'treat/theme' {
  type Tokens = typeof tokens;
  // eslint-disable-next-line @typescript-eslint/no-empty-interface
  export interface Theme extends Tokens {}
}

Implementation ideas for glaze/sx-prop.d.ts:

The recommended .d.ts file should be documented, maybe on a page dedicated to usage with TypeScript.

@kripod
Copy link
Owner

kripod commented Apr 12, 2020

Quite possibly, the useStyling hook's implementation will be split into 2 parts:

  1. Static style resolver, pairing CSS objects to the static class names generated by treat
  2. Dynamic style injector, as an optional fallback

The underlying ThemedStyle type should be reused for the sx prop's typings, but it will possibly be moved to a new location.

@lgtm-com
Copy link

lgtm-com bot commented Apr 17, 2020

This pull request introduces 50 alerts when merging c0d3d67 into 1f79156 - view on LGTM.com

new alerts:

  • 48 for Unused variable, import, function or class
  • 2 for Superfluous trailing arguments

@lgtm-com
Copy link

lgtm-com bot commented Apr 17, 2020

This pull request introduces 50 alerts when merging 2421fd4 into 1f79156 - view on LGTM.com

new alerts:

  • 48 for Unused variable, import, function or class
  • 2 for Superfluous trailing arguments

@kripod
Copy link
Owner

kripod commented Apr 18, 2020

Fantastic work, thank you so much! 🙌

I would appreciate if you could document the plugin's usage and known caveat inside the readme, linking it to #27. Please let me know if you need any kind of assistance!

@kripod kripod changed the title chore(babel-plugin-glaze): add initial plugin feat(babel-plugin): initial version Apr 18, 2020
@kripod kripod merged commit 58d3ef6 into kripod:master Apr 18, 2020
@kripod
Copy link
Owner

kripod commented Apr 18, 2020

@all-contributors please add @tatchi for code and tests

@allcontributors
Copy link
Contributor

@kripod

I've put up a pull request to add @tatchi! 🎉

Repository owner deleted a comment from allcontributors bot Apr 18, 2020
@kripod
Copy link
Owner

kripod commented Apr 20, 2020

I got an even better idea for adding the sx prop types to elements: Create a package called @types/babel-plugin-glaze in DefinitelyTyped and then add it as a dependency of babel-plugin-glaze to auto-import the .d.ts file.

(The work has started at DefinitelyTyped/DefinitelyTyped#44048)

kripod added a commit to kripod/types-publisher that referenced this pull request Apr 20, 2020
Required for seamless augmentation of React attributes, in order to add the `sx` prop for any element: kripod/glaze#16 (comment)
sandersn pushed a commit to microsoft/types-publisher that referenced this pull request Apr 20, 2020
* Add glaze to dependenciesWhitelist

Required for seamless augmentation of React attributes, in order to add the `sx` prop for any element: kripod/glaze#16 (comment)

* Add treat to dependenciesWhitelist
@kripod
Copy link
Owner

kripod commented Apr 20, 2020

The type definitions are now ready to roll, awaiting to be merged into the DefinitelyTyped repository. Once they are ready, adding @types/babel-plugin-glaze to the dependencies field of babel-plugin-glaze will make typings available for the sx prop.

blueskyleader01 added a commit to blueskyleader01/Types-Publisher that referenced this pull request Mar 8, 2024
* Add glaze to dependenciesWhitelist

Required for seamless augmentation of React attributes, in order to add the `sx` prop for any element: kripod/glaze#16 (comment)

* Add treat to dependenciesWhitelist
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