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

Add babel to gulp buildDev workflow for js compilation #412

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

jon-braz
Copy link
Contributor

@jon-braz jon-braz commented Aug 8, 2019

Added babel to allow for ES6+

@jon-braz
Copy link
Contributor Author

jon-braz commented Nov 6, 2019

Hi @azefiel, could I get your thoughts on this? It's just a quick change to the buildProd pipeline to add Babel, usually adds another ~10 seconds to the build time.

@joristirado joristirado self-assigned this Nov 7, 2019
@joristirado
Copy link
Contributor

Hey @jon-braz, apologies, I didn't see this PR until now!

The problem with your implementation is that it only transpiles JavaScript inside the apps.

If we were to enable ES6 syntax/features, we would have to do it for monster libraries as well to keep things consistent throughout the whole framework.

Also, the buildProd task is only meant to build Monster UI Core (vs a specific app), so the transpiling step would also have to be added to the buildApp task.

@jon-braz
Copy link
Contributor Author

jon-braz commented Nov 7, 2019

No worries, thanks for taking a look.

If you're open to the idea then, I can definitely generalize it to work for the monster libs and in the other build pipelines.

@jon-braz
Copy link
Contributor Author

jon-braz commented Nov 8, 2019

I've increased the coverage to include src/js/lib/* as well, but it seems nicest to still exclude src/js/vendor to keep build times down (it would add another ~20 seconds to the build).

How do you feel about keeping babel out of buildDev / jsWatch? I feel extra few seconds to run the watcher might get annoying.

@jon-braz
Copy link
Contributor Author

@azefiel I've made some changes to increase coverage and add it to buildApp, your thoughts?

@joristirado
Copy link
Contributor

@azefiel I've made some changes to increase coverage and add it to buildApp, your thoughts?

Apologies for the lack of reply on this, we are currently pushing for a bunch of initiatives to be out of the door by the end of the year so unfortunately, we can not allocate time to review/test this PR for the time being.

@jon-braz
Copy link
Contributor Author

Alright, whenever you have the chance to look at it down the road I'd appreciate it

@jon-braz
Copy link
Contributor Author

@azefiel If you get the chance, could you let me know if you think you'd be open to moving to using babel and allowing ES6+?

@DustinBrett
Copy link

Hi @azefiel,

I've been tasked with continuing progress on this PR from where @jon-braz left off. Are there any further tweaks you would like from us for this feature?

@dklein1211
Copy link

Hello, I do a lot of front-end development for monster-ui and really looking forward to this getting released. If possible I would be more then willing to assist with getting this PR completed if you have some time to get me up to speed on what needs to be done @DustinBrett @azefiel

@joristirado joristirado removed their assignment Jan 18, 2023
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.

4 participants