AceCentre / pasco

Phrase Auditory Scanning COmmunicator - AAC App for iOS and the Web
https://app.pasco.chat
GNU General Public License v3.0
14 stars 6 forks source link

[WIP] Moving to webpack from bower #223

Closed gavinhenderson closed 4 years ago

gavinhenderson commented 4 years ago

This is a PR for #221

Note this PR is a work in progress, DO NOT MERGE. Just wanted to submit the PR now so that you can track the work being done đź‘Ť

hosseinzoda commented 4 years ago

Hi Gavin I've some comments on integration into webpack.

  1. I've already integrated some parts of javascript codes with webpack and babel-loader, You can find it in gulpfile.js.
  2. Our sass integration is a bit old. Moving it to webpack is a good idea.
  3. We have some scripts for pushing pasco content into cordova/www. This can also benefit from webpack. Since the script (scripts/push-cordova) other than copying files to cordova it also injects script tag and removes some text from html files.

Let me know if you any question.

Thanks

gavinhenderson commented 4 years ago

@hosseinamin Thanks for the pointers!

I have to admit that when it comes to cordova I don't have a lot of experience so might need your help verifying that it all still works.

My goal is to go page by page migrating to webpack, starting with the 'intro' page.

willwade commented 4 years ago

Looks like you are making great efforts! Thanks! Let me know when close to testing this - I/@hosseinamin can check all works on the Cordova/iOS side of things

gavinhenderson commented 4 years ago

Hey @willwade , @hosseinamin . Hope you both are well,

So my progress so far is that I have converted the /intro page and /quick-setup to using the webpack setup. Feel free to checkout my fork and have a poke about, it is mostly contained in the /src directory and the /webpack directory.

I have

The options I see:

  1. Scrap this PR and issue together and continue with bower/gulp as code quality and test coverage seem good enough right now so lets focus on fixing bugs and shipping features.

  2. Create a version that works with both the bower/gulp pages and webpack, each page only living in one setup. This would be nice as it would prevent a long lived branch. The aim would still be to remove the bower/gulp build process in favour of the complete webpack one, but just do it one page at a time. Note, I am still keen to keep the future webpack setup completely separate from the gulp/bower setup. As part of this I would recommend a slower process, refactoring out some usage of the underscore library as webpack means we can write modern JS and compile out to a version old browsers understand. I would also during this migration propose writing some tests. This would essential become somewhat of a rewrite/refactor so I understand if you dont like this idea. Further more I am happy to commit to doing this work

  3. Keep the work in a branch and do a big merge at the end. This approach would be to swap gulp/bower for webpack and modify as little code as possible then refactor the code from there.

My preference is option 2 but I would love to hear your thoughts on this!

Thanks again, Gavin

willwade commented 4 years ago
  1. Makes a lot of sense. Definitely +1 for tests too. We’ve chatted about desperately needing those (we’ve always been hit by breaking functionality that we’ve missed). @hosseinamin - any thoughts? I guess so we need to add anything to the normal build process?
hosseinzoda commented 4 years ago

@gavinhenderson I like your work. If you can can build the webpack version successfully. And it is in working condition. Lets put gulp/bower build system in trash and move to webpack. On existing builds, Mainly sass and jsx is built in gulp. For sass there are three outputs common.css, main.css and edit-config.css. Which you can combine all three in one file for simplification. And on jsx, nodelib.js. Which you can simply move it to new webpack build. Also on testing, Having tests is very useful.

willwade commented 4 years ago

Gavin. Totally realised something after reviewing my comment. I meant go with 2 not 1. I swear I wrote 2 but maybe markdown parser screwed that up!!! My apologies. Love what you’ve been doing!!

willwade commented 4 years ago

Gavin. Totally realised something after reviewing my comment. I meant go with 2 not 1. I swear I wrote 2 but maybe markdown parser screwed that up!!! My apologies. Love what you’ve been doing!!

Oh man - something odd with all my github apps. On iPhone it was saying I wrote 1.. Just ignore my inane chat

gavinhenderson commented 4 years ago

Hey @willwade this is ready to be merged now. For now its just the intro page that is migrated to the webpack build.

The webpack build currently has just been built into the /html folder so that it integrates with the current build process. Eventually we will be in a place where we have a dist folder that will not be committed to the repo.

Once this PR has been merged ill open a new one to migrate the next page

willwade commented 4 years ago

Thanks. I just need to check all the iOS build is working ok and there is no onward knock on problems. May merge to a new branch and work on that. Big thanks Gavin