search

maplibre / maputnik

An open source visual editor for the 'MapLibre Style Specification'
https://www.maplibre.org/maputnik
MIT License
2.14k stars 402 forks source link

Replace webpack with Vite #803

Closed damianstasik closed 11 months ago

damianstasik commented 1 year ago

Hello 👋

Would you accept a PR that replaces webpack with Vite? It's a modern build tool that would make developer experience much better, it would also greatly reduce amount of dependencies and simplify the project setup. I did some initial tests and I was able to switch Maputnik to Vite fairly smoothly, here is the result: https://github.com/maputnik/editor/compare/master...damianstasik:editor:replace-webpack-with-vite (just look at that package-lock.json reduction 🤩).

I would love to contribute such a PR, but what are your thoughts on that?

HarelM commented 11 months ago

Disclaimer: I'm not maintaining this project.

I've tried to introduce some typescript migration ability in webpack and it was frustrating to say the least. What I thought would be a good way forward was to slowly change the jsx files to tsx, bit by bit so it won't be an all-or-nothing kind of effort. How complicated would it be to do that in vite? Currently the solution I found was that I need to change the import statements from import a from './a' to import a from './a.tsx' which feels like a hack and is not needed if everything is written in typescript, so there's an extra effort in the end which I would like to avoid, and webpack configuration doesn't seem play nice for some reason.

So if this can be a way forward maybe a joint effort here can be a good idea, what do you think?

P.S. just saw that this issue is from Feb :-/

damianstasik commented 11 months ago

Hey @HarelM!

I've tried to introduce some typescript migration ability in webpack and it was frustrating to say the least. (...) How complicated would it be to do that in vite?

I'd say it's one of the reasons Vite became so popular – just add tsconfig.json config and install typescript, that's it, TS is enabled.

webpack configuration doesn't seem play nice for some reason

Could you see if adding .ts and .tsx to the resolve.extensions option helps with this?

So if this can be a way forward maybe a joint effort here can be a good idea, what do you think?

I agree, just one question, do we know if such a migration would be merged?

HarelM commented 11 months ago

Could you see if adding .ts and .tsx to the resolve.extensions option helps with this?

It does help, but it seems like an intermidiate hack, and I don't like it...

I agree, just one question, do we know if such a migration would be merged?

You can see the comment in #828 I believe it will be welcomed.

I also did some experimentation with ts-migrate tool, which does a lot of the manual annoying stuff, it leaves a lot of "TODOs", but this is expected.

I would recommend opening a PR just for vite and see how it goes, after that we can see how we can upgrade the dev dependencies and introduce typescript. After that we can start working on removing the "TODOs" that are related to typescript (which I don't think is a huge task as the components have static propTypes) and finally upgrade the dependencies with latest packages.

I can reach out to the maintainers to give us contributor roles in needed and if this is something you would like to maintain going forward.

HarelM commented 11 months ago

I have opened a PR with changes that are very similar to what is linked here. Once merged we could incrementally change the code from javascript to typescript.