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

admin/vite-build #154

Merged
merged 6 commits into from
Dec 31, 2021
Merged

Conversation

tohuynh
Copy link
Collaborator

@tohuynh tohuynh commented Dec 30, 2021

Link to Relevant Issue

WIP #134

Description of Changes

Build the app and lib using vitejs

npm commands for the app

  • npm run start:app launches the app locally
  • npm run build:app builds the app in production mode. Generates static site in the build dir. See an example build dir and example deployment.
  • npm run deploy:app deploys build dir (the example app) to forked repo's gh-pages
  • npm run preview:app launches the app locally, but in production mode at localhost:5000/cdp-frontend

build lib

npm run build:lib builds cdp-frontend as a library to be consumed by various CDP instances by following these recommendations. It generates a dist dir with 3 files: index.es.js, index.umd.js, and index.css (the assets are inline in the minified CSS file)

misc changes

  • Removed webpack and related dev dependencies. I had to perform a fresh npm i to make it work.
  • @BrianL3 I had to change the use of the default cover images a little bit so that they can be imported dynamically in order for them to be included in the final build.

Copy link
Collaborator

@BrianL3 BrianL3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only comment is that you should maybe update the README/CONTRIBUTION to make sure people know that npm run start:app and npm run build:app are the ways to get the app up and running. I would like Nick McGruder and other devs new to the project to have a seamless experience getting the app up and running.

@tohuynh
Copy link
Collaborator Author

tohuynh commented Dec 30, 2021

My only comment is that you should maybe update the README/CONTRIBUTION to make sure people know that npm run start:app and npm run build:app are the ways to get the app up and running. I would like Nick McGruder and other devs new to the project to have a seamless experience getting the app up and running.

Good point

@tohuynh
Copy link
Collaborator Author

tohuynh commented Dec 30, 2021

Added some documentation for new npm commands to contributing.md

Added another command npm run deploy:app which deploys the example app to your forked repo's gh-pages. Devs can choose to deploy the storybook site or the example app, when making a PR.

Copy link
Collaborator

@hawkticehurst hawkticehurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beyond a question about Storybook still working everything looks great!

package.json Show resolved Hide resolved
@tohuynh tohuynh linked an issue Dec 30, 2021 that may be closed by this pull request
Copy link
Member

@evamaxfield evamaxfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Can someone give me the high level overview of what are the benefits of switching?

@tohuynh
Copy link
Collaborator Author

tohuynh commented Dec 31, 2021

Looks good to me! Can someone give me the high level overview of what are the benefits of switching?

  • Build app and lib with the same frontend tooling
  • build:app is now a part GitHub CI
  • Faster dev server with hot module reload (with cdp icons rendering, snowpack wasn't working before)
  • why Vite?
  • some build optimization

@evamaxfield
Copy link
Member

Beautiful. Thank you.

@tohuynh tohuynh merged commit aa0e015 into CouncilDataProject:main Dec 31, 2021
@tohuynh tohuynh deleted the feature/vite-build branch December 31, 2021 22:25
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.

Icons are from cdp-design and Mozilla protocol aren't rendering
4 participants