BlazorExtensions / BlazorMaterial

Blazor components implementing Google's Material components for web - https://material.io/components/web
MIT License
135 stars 23 forks source link

Upgrade from 0.3.0 to 0.7.0 #4

Closed acherkashin closed 5 years ago

acherkashin commented 6 years ago

In new 0.5.1 version:

larsselle commented 6 years ago

@galvesribeiro you wrote, that we don't want to have multiple attached functions in global scope. That's good. You have linked an example to a TypeScript-File in SignalR. Can we use in this project JavaScript or is TypeScript the favorite?

For JavaScript we don't need a transpiler and the build process is easy.

What do you think about this?

galvesribeiro commented 6 years ago

@larsselle

If you meant for the contributors yes, we should use TS. All the projects on BlazorExtensions if they are doing interop, it should go thru typescript. It has a lot more tooling support for new contributors and a lot of runtime failures that JavaScript may present to use are got by typescript at development time so we decided to use it as a standard everywhere.

If you meant that it is a required for consumers of this package, no, you don't need typescript to consume it.

The .JS projects on all the projects under this Org are configured to transpile properly the TS code and embed the output on the .JS dll so there is not much effort for anyone. As long as you have nodejs installed on your dev machine it should do its magic but again, it is just in case you are contributing to any of those repos. You don't need that to consume it.

larsselle commented 6 years ago

I meant for the contributors. Today i don't have any TS experience. I will learn and test a liitle bit for me, so that i can contribute to this project.

Thank you for your reply.

galvesribeiro commented 6 years ago

Thanks for understand @larsselle

We try keep all TS as simpler as possible and closer to ES6 classes itself without use too much fancy features as much as we can.

We also created the .JS projects in a way that it is mostly automatic the transpilation process so people don't have to care about that.

acherkashin commented 6 years ago

@galvesribeiro I have looked on your feedback and make changes. I added to .gitignore global.json and launchSettings.json files. Also, I got rid of js wrapper functions because we can call methods of mdc library directly from C#.

larsselle commented 6 years ago

@acherkashin The direct call to mdc is a good idea, but i think we have a little problem. At this time, nobody can see the used version of mdc. We must add package.json, webpack for bundle and the other things to the projects.

@galvesribeiro Is it okay when we add the files (package.json, wbpack.config.js, tsconfig.json, tsfmt.json, tslint.json) to the existing projects? I think it is better when we not add extra .JS projects, because any project is a request from the browser to the server. At this time, we have eight projects, so browser do eight requests. When we add the .JS projects, the browser need sixteen requests to the server to load all the things to run the app. I think, less request are better. I can change this, when you accept this pull request.

galvesribeiro commented 6 years ago

@larsselle I know we have many projects as we decided to split the project on smaller, per component ones.

For now we will keep it that way as we are looking of ways to use ILMerge so they will all be a single thing.

acherkashin commented 6 years ago

@galvesribeiro Thanks for your notice. I changed calling OnAfterRenderAsync to OnAfterRenderAsync.

larsselle commented 6 years ago

@galvesribeiro That we have one project for each component is ok. What i mean is, that we must add the package.json, webpack and so on to this projects, because nobody knows where come the mdc JS and CSS files and which versions we use.

For this things, i would not add extra .JS projects for each component. For example: BlazorMaterial.Base and BlazorMaterial.Base.JS or BlazorMaterial.Button and BlazorMaterial.Button.JS

I think we can add JS, CSS, TS and C# for each component in a single projects for the component. That is what i would change in the next pull request after this.

Sorry for my bad expression.

acherkashin commented 6 years ago

Hi, @galvesribeiro. Can you accept this pull request? Or maybe do you want I make other changes?

galvesribeiro commented 6 years ago

@acherkashin looks better.

Can you also update to Blazor 0.6.0?

Also, more and more you are moving to what @larsselle suggested... To ditch the .js projects.

I'm not against it if we:

  1. For non-static components, we have a way to interop directly with the component and maintain the instance object on the C# side without any JS.
  2. For static components we make clear that the methods are static on the C# side or they are extension methods of something (i.e. the ElementRef instance)
  3. In any case we shouldn't have any JS/TS/CSS on the C# project. For the cases that require us to add that, it should be on the .JS project. I don't want the C# project to contains those things as they are supposed to (in the future) be a clear C# implementation of the component and not depend on JS anymore.
acherkashin commented 6 years ago

@galvesribeiro Sure, I am going to update Blazor to 6.0 version.

acherkashin commented 6 years ago

@galvesribeiro I upgrated blazor to 6 version.

acherkashin commented 6 years ago

@galvesribeiro

What do you think about rewriting JS logic to C#? Will be it really difficult or not?

galvesribeiro commented 6 years ago

What do you think about rewriting JS logic to C#? Will be it really difficult or not?

Lets discuss that after this PR. Jump on BlazorExtensions channel once we finish here and ping me there.

acherkashin commented 6 years ago

@galvesribeiro Blazor.Cli is updated

acherkashin commented 5 years ago

@galvesribeiro Do you have another remarks?

galvesribeiro commented 5 years ago

Sorry for the delay @acherkashin

Now that we have 0.7.0, can we have it updated to it? As soon as you get it I'll merge.

Thanks

acherkashin commented 5 years ago

@galvesribeiro Blazor is updated

acherkashin commented 5 years ago

Hi, @galvesribeiro . Can you accept this pull request?

galvesribeiro commented 5 years ago

Thank you @acherkashin for your patience. It is in now.