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

Fix GPA Calc by adding IIFE. Add Webpack to bundle NPM Packages #27

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

RachitKeertiDas
Copy link
Contributor

@RachitKeertiDas RachitKeertiDas commented Dec 6, 2020

It would be helpful to allow using NPM Packages using a bundler like Webpack.

Webpack needs to be set up to bundle the .js files with the imported packages.
To allow for easy installation, I would recommend adding a post install hook to run webpack immediately after clone.
Also, we could set up a pre-commit hook to check that commits build successfully.

Task List

  • Set Up Webpack build targets. Link the html files to source the built JS Files.
  • Add a Post Install Hook to allow for easier installation.
  • Modify Pre-Commit Hook to include webpack.
  • Change Documentation to reflect webpack changes.
  • Test PDF Libraries with Webpack.

This PR Is currently a WIP.

  • I've currently set up a separate target for each section (popup/gpa/timetable), and changed the html files to source the built files.

Please review and provide suggestions.

Update:

  • I've completed the tasks required. Unmarking The WIP status. Please review.

Update2:

  • Currently there's a regression wrt the downloaded pdf being corrupt. So marking it as WIP

This allows script to run multiple times on the same page without errors.
Add Webpack to allow use of NPM Modules
Use Webpack config to specify a target for the each HTML page.
npx webpack builds the pages into build/
Modified HTML pages to link to js files in build/

Modify .gitignore to include build/
Add Post-Install Hooks for automatic build on install.
Modify Pre-Commit Hook to check for successful builds.

Modify Installation Instructions to include `npm install`.
Add Note about Development Instructions.
@RachitKeertiDas RachitKeertiDas marked this pull request as ready for review December 6, 2020 18:18
@RachitKeertiDas RachitKeertiDas marked this pull request as draft December 9, 2020 05:29
@npalladium
Copy link
Collaborator

@RachitKeertiDas?

@RachitKeertiDas
Copy link
Contributor Author

Ah yes, this.

I have been slightly busy on this. Also haven't been able to figure out the regression yet.
Feel free to checkout/push more commits.
I might be able to try and fix before the new year.

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.

2 participants