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.

Sign up for GitHub

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

Jump to bottom

Vite3 rebase nov22 #11281

Merged
merged 4 commits into from
Dec 18, 2023
Merged

Vite3 rebase nov22 #11281

merged 4 commits into from
Dec 18, 2023

Conversation

happythenewsad
Copy link
Contributor

@happythenewsad happythenewsad commented Nov 25, 2023 •
edited
Loading

WHAT

  • replaces webpacker with vite in all environments

WHY

  • we want a better featured and faster front end build environment

HOW

  • remove webpacker artifacts
  • add vite artifacts and configs
  • add various shims and syntax improvements to the codebase, since the Vite build is less tolerant of older syntaxes (commonJS style imports, for example)

Help, this is huge, where do I start?

  1. All Vite configs are handled by two files: vite.json and vite.config.ts. Reading those is a good place to start.
  2. Vite organizes js and css into entrypoints. These are analogous to webpack's 'bundles.' Scroll through client/app/entrypoints to get a feel for the structure.
  3. The QA and rollout plan can be found here: https://www.notion.so/quill/Vite-create-QA-and-rollout-checklists-10c64bb2ed154331a36f6e2f7c7f91bb
  4. Unsquashed commit history is partially preserved in these branches:
vite3-rebase-dec14-unsquashed
vite3-rebase-nov22-unsquashed
vite3-aug29-heroku-working-archive
PR Checklist Your Answer
Have you added and/or updated tests? Yes
Have you deployed to Staging? Yes
Self-Review: Have you done an initial self-review of the code below on Github? yes
Spec Review: Have you reviewed the spec and ensured this meets requirements and/or matches design mockups? n/a

@happythenewsad happythenewsad force-pushed the vite3-rebase-nov22 branch 2 times, most recently from 3044b05 to 37d9c22 Compare November 28, 2023 20:16
@happythenewsad happythenewsad force-pushed the vite3-rebase-nov22 branch 2 times, most recently from 61d9067 to 99d33fd Compare November 29, 2023 17:47
@happythenewsad happythenewsad force-pushed the vite3-rebase-nov22 branch 3 times, most recently from 5dc8278 to dc04012 Compare December 7, 2023 19:26
Peter Kong added 2 commits December 17, 2023 20:20
freshening vite3 via merge --squash strategy
bd54d7c 
This commit contains all vite work as of today:
- vite generally serving the entire app without error
- all jests passing
- reversion to using process.env instead of import.meta.env

The unsquashed git history may be accessed via the vite3-june14-bak branch.
Latest commit in this squash: ba9fd

re-introduce vite shim, which was clobbered during a recent rebase

Vite3 prod build (#10896)

* update lockfiles

temporarily deleting entrypoints for debugging

Revert "temporarily deleting entrypoints for debugging"

This reverts commit d26e9a17aa82b9fd83f557e063cd376716cc5b7c.

removing comments

include airbyte script from develop branch

* our build doesn't support brackets in react components, so re-syntaxing the code

* move from cjs to esm syntax in particular file to fix build

* this config has a working build

* remove console log

* parameterize env vars

---------

Co-authored-by: Peter Kong <[email protected]>

remove webpack/ and webpacker.yml

comment out initializers/react_on_rails.rb

- removed webpacker gem \n - removed webpacker binstubs \n - removed webpack specific scripts in package.json

import vairables in cms_table.scss

Revert "import vairables in cms_table.scss"

This reverts commit 80ea53b.

remove cms.css from config.assets.precompile

removing obsolete Procfiles and webpack configs

Procfile config change; react_on_rails initializer config change

remove print statements

pin vcr gem; remove commented statements in erb file

CMS_URL -> QUILL_CMS

whitespace

freshen package.json

update package-locks

update Gemfiles

fix single spec regression

remove referenced to deleted files edit-assigned-students, final_score_page

fix SegmentAnalytics syntax issue, remove old css file reference, to get prod build working again

update snapshots

update snap

revert bin/bundle changes

lint

relocking Gemfile after bundle lock --add-platform x86_64-linux

silence system spec that depends on webpacker environment

add modified bundler binstub so that local env starts

Gemfile.lock updates

bundle lock --add-platform x86_64-linux

removing require statements from js files

check in new package lockfiles

regressions

add evidence scss entrypoint

removing linux platform from lockfile

adding mini_racer gem

delete unused webpacker-dependent specfile; linting

add ulimit to avoid too many files error during local serving

update moment import syntax to avoid 'moment is not a function' errors

update setupTets, tsconfig so that momentjs plays nice with jest

fix failing node specs, part 1

fix failing node specs, part 2

removing jest mock from setupTests

more jest fixes

concert C = require( statements to import statements

convert more require statements to import

Pin webmock (#11300)

fix test (#11302)

fix activity assignment flow null value bug

fix mock- and snapshot- related jest failures

fix eslint

update snapshots, remove one-off issue in conceptResults.test.ts

update jest snapshots

fix more jests

fix Pusher import statement

resolve merge conflicts

fix view diagn report

topic_filters bugfix

debug

const moment = require('moment'); -> import moment from 'moment';

introduce new stylehseets to premium_hub/index.scss

add entrypoints.session.ts for signup flow

import underscore lib in various places; add empty shared.ts endpoint

add contacts.scss to scss index file

import lodash wherever '_' is referenced

specify correct entrypoint bundle for /premium

vite frontend bug fixes (#11349)

* remove home import from teacher bundle

* add scoring updates card to scss index

* import home file in the right place in application.scss

* fix bug where activity counts aren't showing up

* add back header margins to style guide

* fix grade level filters file

* add update-assigned-student scss to pages/index

* move locker into staff bundle so it can get the right css

* fix evidence css

* add proofreader scss entrypoint

* fix tests

* one more test

* remove require in favor of regular import

* fix merge conflict

change env var back (#11352)

rebuilding package-lock.json after rebase

fix non-react javascript for Vite (#11355)

* freshening vite3 via merge --squash strategy

This commit contains all vite work as of today:
- vite generally serving the entire app without error
- all jests passing
- reversion to using process.env instead of import.meta.env

The unsquashed git history may be accessed via the vite3-june14-bak branch.
Latest commit in this squash: ba9fd

re-introduce vite shim, which was clobbered during a recent rebase

Vite3 prod build (#10896)

* update lockfiles

temporarily deleting entrypoints for debugging

Revert "temporarily deleting entrypoints for debugging"

This reverts commit d26e9a17aa82b9fd83f557e063cd376716cc5b7c.

removing comments

include airbyte script from develop branch

* our build doesn't support brackets in react components, so re-syntaxing the code

* move from cjs to esm syntax in particular file to fix build

* this config has a working build

* remove console log

* parameterize env vars

---------

Co-authored-by: Peter Kong <[email protected]>

remove webpack/ and webpacker.yml

comment out initializers/react_on_rails.rb

- removed webpacker gem \n - removed webpacker binstubs \n - removed webpack specific scripts in package.json

import vairables in cms_table.scss

Revert "import vairables in cms_table.scss"

This reverts commit 80ea53b.

remove cms.css from config.assets.precompile

removing obsolete Procfiles and webpack configs

Procfile config change; react_on_rails initializer config change

remove print statements

pin vcr gem; remove commented statements in erb file

CMS_URL -> QUILL_CMS

whitespace

freshen package.json

update package-locks

update Gemfiles

fix single spec regression

remove referenced to deleted files edit-assigned-students, final_score_page

fix SegmentAnalytics syntax issue, remove old css file reference, to get prod build working again

update snapshots

update snap

revert bin/bundle changes

lint

relocking Gemfile after bundle lock --add-platform x86_64-linux

silence system spec that depends on webpacker environment

add modified bundler binstub so that local env starts

Gemfile.lock updates

bundle lock --add-platform x86_64-linux

removing require statements from js files

check in new package lockfiles

* regressions

add evidence scss entrypoint

removing linux platform from lockfile

adding mini_racer gem

delete unused webpacker-dependent specfile; linting

add ulimit to avoid too many files error during local serving

update moment import syntax to avoid 'moment is not a function' errors

update setupTets, tsconfig so that momentjs plays nice with jest

fix failing node specs, part 1

fix failing node specs, part 2

removing jest mock from setupTests

more jest fixes

concert C = require( statements to import statements

convert more require statements to import

Pin webmock (#11300)

fix test (#11302)

fix activity assignment flow null value bug

fix mock- and snapshot- related jest failures

fix eslint

update snapshots, remove one-off issue in conceptResults.test.ts

update jest snapshots

fix more jests

fix Pusher import statement

* resolve merge conflicts

* fix view diagn report

* topic_filters bugfix

* debug

* const moment = require('moment'); -> import moment from 'moment';

* introduce new stylehseets to premium_hub/index.scss

* add entrypoints.session.ts for signup flow

* import underscore lib in various places; add empty shared.ts endpoint

* add contacts.scss to scss index file

* import lodash wherever '_' is referenced

* specify correct entrypoint bundle for /premium

* vite frontend bug fixes (#11349)

* remove home import from teacher bundle

* add scoring updates card to scss index

* import home file in the right place in application.scss

* fix bug where activity counts aren't showing up

* add back header margins to style guide

* fix grade level filters file

* add update-assigned-student scss to pages/index

* move locker into staff bundle so it can get the right css

* fix evidence css

* add proofreader scss entrypoint

* fix tests

* one more test

* remove require in favor of regular import

* fix merge conflict

* wip

* fix spinners

* remove unnecessary javascript addition from application file

* fix package locks

* undo changes to database.yml and puma.rb file

* remove whitespace

* remove whitespace from database.yml

* fix whitespace in puma.rb

* one more time

---------

Co-authored-by: Peter Kong <[email protected]>

add media:all directive to stylesheet tag so print media works
resolving merge conflicts in lock files
974da48
@happythenewsad happythenewsad marked this pull request as ready for review December 18, 2023 03:40
emilia-friedberg
emilia-friedberg requested changes Dec 18, 2023
View reviewed changes
Copy link
Member

@emilia-friedberg emilia-friedberg left a comment

Choose a reason for hiding this comment

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

Overall, looks great to me. I left comments in a couple of places I think we could clean up, but those are very non-urgent and can be done in a follow-up. Requesting changes just to ensure the Lessons ENV variable gets fixed.

services/QuillLMS/Procfile.prod_cms Outdated
@@ -5,13 +5,17 @@ worker: bundle exec sidekiq -v -q default -q critical
reportworker: bundle exec sidekiq -v -q low

# Build client assets, watching for changes.
rails-client-assets: sh -c 'npm run build:dev:client:prod-cms'
#rails-client-assets: sh -c 'npm run build:dev:client:prod-cms'
Copy link
Member

Choose a reason for hiding this comment

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

Probably something we can do as a follow-up, but we should probably remove these commented-out lines (and restore parity re: using the production CMS_URL env variable, since right now this file is the same as Procfile.static).

services/QuillLMS/app/assets/stylesheets/application.scss
@@ -1,12 +1,12 @@
@import "../../../client/node_modules/react-datetime/css/react-datetime";

@import "variables";
@import "bootstrap-sprockets";
@import 'variables';
Copy link
Member

Choose a reason for hiding this comment

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

This isn't really a code comment, but just a general one that it is probably worth sending out a note about the major structural changes (like having to explicitly import every nested scss file) to quill-developer whenever this goes out.

services/QuillLMS/client/app/bundles/Connect/libs/question.js Outdated Show resolved Hide resolved
services/QuillLMS/client/app/bundles/Evidence/styles/style.scss
@@ -16,7 +16,10 @@
@import './activityFollowUp.scss';
@import './shared.scss';
@import '../../../../../app/assets/stylesheets/shared/teacher-preview-menu.scss';
@import "~bulma/bulma.sass";
Copy link
Member

Choose a reason for hiding this comment

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

This removal (not sure exactly when this happened) probably accounts for why we needed to add the lines below (as caught during QA). If that was literally the only thing it was affecting (which is very possible), we should leave as is, but just flagging this in case we discover more visual bugs down the line.

services/QuillLMS/client/app/bundles/Lessons/utils/socketStore.js Outdated Show resolved Hide resolved
services/QuillLMS/client/app/bundles/Shared/styles/styles.scss Outdated Show resolved Hide resolved
.../QuillLMS/client/app/bundles/Teacher/components/accounts/new/__tests__/verify_email.test.tsx Outdated Show resolved Hide resolved
services/QuillLMS/client/setupTests.ts Outdated Show resolved Hide resolved
@happythenewsad happythenewsad temporarily deployed to empirical-grammar-staging December 18, 2023 16:44 Inactive
@happythenewsad happythenewsad merged commit ed03b60 into develop Dec 18, 2023
14 checks passed
@delete-merged-branch delete-merged-branch bot deleted the vite3-rebase-nov22 branch December 18, 2023 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emilia-friedberg emilia-friedberg emilia-friedberg approved these changes

@dandrabik dandrabik Awaiting requested review from dandrabik

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@happythenewsad @emilia-friedberg