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

Typescript port progression Iro.JS #80

Closed
KaanMol opened this issue Jun 13, 2019 · 14 comments
Closed

Typescript port progression Iro.JS #80

KaanMol opened this issue Jun 13, 2019 · 14 comments

Comments

@KaanMol
Copy link

KaanMol commented Jun 13, 2019

This issue is a follow-up of the conversation in #68.

@KaanMol
Copy link
Author

KaanMol commented Jun 13, 2019

Okay currently Iro.JS is made in preact, that is not a problem for the vanilla versions. But I saw atleast 1 issue for a ReactNative port. We talked about what you really wanted with Iro.JS, you answered that you wanted to make this a core library with just the basics.
What if we even remove the UI and make Iro.JS literally just a core and then make the UI a sort of layer on top of that. Why? Because then you should be able to make a clean port to React, Vue, Angular etc (if needed ofcourse).
I still have to think it out about how to make it etc. It is still just an idea.

@jaames
Copy link
Owner

jaames commented Jun 13, 2019

Yeah, I've flirted with the "split core" idea a few times. It would be amazing if we could pull that off; I think the lack of seamless integration with popular frameworks like React, Vue, etc (at least, without hacky wrappers like in #54 ) is a considerable drawback right now

In the past I was never quite sure where to draw the line between what should and shouldn't be in the "core", and what to do with plugins to make sure they work across various hypothetical ports. There also still needs to be a vanilla version with a pre-bundled Preact UI that can used with minimal fuss

It will need some careful planning, but it would definitely take the library to the next level

@KaanMol
Copy link
Author

KaanMol commented Jun 14, 2019

We could pull that off easily, it is just a matter of discussing where we indeed draw the line. It should be as easy as just remove preact and the things which aren't gone, is the core.

It is an issue indeed, because, who is going to build the UI? Are we going to give the layout wrapper something like an array with objects and let it build for us. Is the core gonna provide even layout information? We have to think about how and what we really want (and need).

About the vanilla js version, I was thinking to see that as its own wrapper that uses the core of Iron.JS, just like the other wrappers will :)

@jaames
Copy link
Owner

jaames commented Jun 14, 2019

I see two viable options for the split core idea, but theres drawbacks for each

The first option is that iro-core essentially handles everything besides rendering and events. Components like wheel, slider, etc would return virtual DOM trees (which is what they do right now anyway) and then the wrappers have to convert those trees into something they can actually render and bind events to. This would allow wrappers to be much more consistent, but the downside is the vdom tree may not be even remotely compatible with certain target platforms and any changes to the vdom tree structure in iro-core may inadvertently break a wrapper

The second option is to have the wrappers decide how to put the UI together, and iro-core just provides the component options and maybe some generic methods to convert input coordinates to a color and to get the gradient color stops for a slider, etc. This would provide more flexibility for the wrappers, since they can render however they like, but the drawback is that all wrappers may need to be updated when a new feature comes out, and iro plugins wouldn't be able to provide extra components that are compatible with every wrapper

@KaanMol
Copy link
Author

KaanMol commented Jun 14, 2019

So I looked at a few frameworks how they exactly want to handle their events.
Most of the time they just call a function. That would mean that we need to "close" Iro.JS probably.
With closing I mean that the eventlisteners are only for Iro.JS itself and the wrapper accepts hooks which will get called, since accessing the DOM in Angular, React, Preact etc is seen as a bad habit.

About the layout, yeah maybe removing it isn't the best idea then. Because we can't probably make something that dynamic (well, we can, but it wont be as fast and will get too complicated for nothing).
Our only way to make it 100% for just 1 platform is to make a wrapper which handles the layout in every framework...

@jaames
Copy link
Owner

jaames commented Jun 16, 2019

iro.js does add events directly to the DOM, but it should be simple enough to change that. Actually, rather than extending Preact's component class it might help to wrap each of the color picker components in a higher order component (like I suggested back in the PR thread). The higher order component could then provide generic event handlers, etc to each component?

I'll have to think about the split core thing a little more. My concern is that I don't really want the responsibility of maintaining multiple framework wrappers if they all handle layout separately, I think that's a lot of extra work in the long-term :/

Anyway, I was reflecting on this over the weekend. I think we should focus on completing the port to Typescript to the point where we're both happy that the code is clean, well-organised, etc. Once it's done we'll have a solid base to work from for splitting the core if we do decide to go that route

I have to focus on work stuff this week, but after that I'll start contributing to this a little more!

@KaanMol
Copy link
Author

KaanMol commented Jun 17, 2019

Yeah you are right, let's make it Typescript first then. I am free today so I will remove webpack and add microbundle. Clean up some additional things etc, shouldn't be too much work, most of it is done anyway.

I get your concern about the splitted core, but the thing is that it doesn't mean that the repo should be yours, it can be maintained by the community. They probably know better what and how you should make components for that framework.

I just have one small thing and that is that Iro.JS is made in Preact, but it isn't a Preact component. How are we going to do that? Should I just split it? That you have to import something like '@iro.js/preact' to get the component. And importing '@iro.js' gives the component with a vanilla js wrapper

@jaames
Copy link
Owner

jaames commented Jun 17, 2019

I'd prefer to add Typescript compilation to the current rollup build setup, I took a look at microbunble and it doesn't have as much flexibility. I'm happy to work on that myself though, so don't worry about it :)

Hm, yeah, I suppose I should let the community have more responsibility. I kinda have a habit of trying to do everything myself :")

That's something we can look at, sure

@KaanMol
Copy link
Author

KaanMol commented Jun 17, 2019

Sure, if you need help, just ask :)

I know that feeling, I do the same hahah. Sometimes it is easier to try everything yourself.

I am going to start now then :)

@jaames
Copy link
Owner

jaames commented Jun 29, 2019

Sorry for the dead air again, I've been really busy.

I managed to add typescript support to the rollup build script! Although for some reason it is generating seperate .d.ts files for every tsx file in the /src directory -- any idea why?

@KaanMol
Copy link
Author

KaanMol commented Jun 29, 2019

Hey no problem!
I fixed a few small things, but I haven't made huge changes to the source.
It seems like all the functions work, it may be messy etc. You can check it out on my fork.

About your question, is it only for the .tsx files or also for the .ts files?
It shouldn't generate any .d.ts files in your /src folder, they should be in /built.
Do we share the same tsconfig.json?

Maybe it is a good idea to refactor the code together, doing that here will be painfully slow, can I contact you on discord or etc?

@KaanMol
Copy link
Author

KaanMol commented Jun 29, 2019

I looked at the last commits you made and I see that you probably meant the /dist folder (I saw your change from built to dist).
That is normal for typescript, usually this is preferred. I will add a change to the config so that the types will get their own folder. You can then reference this folder in the package.json file.

You probably expected 1 single .d.ts file, you can combine it, but it isn't really needed. The correct .d.ts files will be loaded by the editors.

@jaames
Copy link
Owner

jaames commented Jun 29, 2019

Yeah, the only tsconfig change I made was swapping /built with /dist, since that is the build directory that iro.js currently uses for bundled files. The typedefs are in there, which isn't a problem, but I was hoping that they'd be bundled into a single file. I suppose it isn't an issue, it just feels cleaner to me if it's one file.

Also, can reach me on Discord at @jaames#9860, or on Twitter https://twitter.com/rakujira (DMs should be open) :)

@jaames
Copy link
Owner

jaames commented Oct 21, 2019

Going to close this since the Typescript port is pretty much done, now I'm just working on adding other 5.0 features :)

Thanks for the help! I'll make sure to credit you

@jaames jaames closed this as completed Oct 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants