Skip to content

Conversation

dplewis
Copy link
Member

@dplewis dplewis commented Apr 3, 2025

Pull Request

Issue

Vite is more modern than browserify and has better node polyfilling than standalone rollup.

Closes: #2410
Closes: #2425

Approach

  • Adds umd build for browser script tag
  • Remove gulp-browserify

Limitations

Bundle size and build speed has increased. There are PR's opened to remove dependencies which should significantly reduce the size.

Before:

dist/parse.js 988 kB
dist/parse.min.js 396 kB
dist/parse.weapp.js 988 kB	
dist/parse.weapp.min.js 396 kB	

After:

dist/parse.js  2,110.77 kB │ gzip: 384.42 kB
dist/parse.min.js  946.53 kB │ gzip: 267.08 kB
dist/parse.weapp.js  2,110.81 kB │ gzip: 384.28 kB
dist/parse.weapp.min.js  946.32 kB │ gzip: 267.00 kB

Shoutout to @dblythy for getting started on this!

Copy link

parse-github-assistant bot commented Apr 3, 2025

🚀 Thanks for opening this pull request!

Copy link

codecov bot commented Apr 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (cfb25ef) to head (3b915f3).
Report is 2 commits behind head on alpha.

Additional details and impacted files
@@            Coverage Diff            @@
##             alpha     #2553   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           64        64           
  Lines         6238      6238           
  Branches      1465      1466    +1     
=========================================
  Hits          6238      6238           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dplewis dplewis requested a review from a team April 3, 2025 05:53
@dplewis
Copy link
Member Author

dplewis commented Apr 3, 2025

@mtrezza @dblythy This is ready for review.

@dplewis dplewis requested a review from mtrezza April 4, 2025 11:06
@mtrezza
Copy link
Member

mtrezza commented Apr 6, 2025

@dplewis Does this have an benefit for the developer, or is that merely an internal change?

@mtrezza
Copy link
Member

mtrezza commented Apr 6, 2025

The question was to write a meaningful change log entry for developers. If you are curious to follow the trail of the issue, then you'll see that it has been opened as a solution to another issue. That other issue is just a refactor, but since this was now a larger change I'm asking whether there is anything relevant for developers to know about it that should go into the change log.

@dblythy
Copy link
Member

dblythy commented Apr 9, 2025

@dplewis thanks for picking this up! What needs to be done before merging? Full e2e test?

@mtrezza
Copy link
Member

mtrezza commented Apr 9, 2025

Dom't we already have a e2e test with puppeteer?

@dplewis
Copy link
Member Author

dplewis commented Apr 14, 2025

Does this have an benefit for the developer, or is that merely an internal change?

@mtrezza In the future it should allow for tree shaking, I'm not sure if allows for it now as we will have to write tests for it. I can try to write one for it after this is merged.

Full e2e test?

@dblythy You wrote it lol

for (const fileName of ['parse.js', 'parse.min.js']) {
describe(`Parse Dist Test ${fileName}`, () => {

@dplewis
Copy link
Member Author

dplewis commented Apr 14, 2025

This is ready for review.

@dplewis
Copy link
Member Author

dplewis commented Apr 14, 2025

FYI if #2501 is merged the build size will be reduced ~16%.

Before

dist/parse.js 988 kB
dist/parse.min.js 396 kB
dist/parse.weapp.js 988 kB	
dist/parse.weapp.min.js 396 kB	

After

dist/parse.js  833.04 kB │ gzip: 156.70 kB
dist/parse.min.js  307.67 kB │ gzip: 88.86 kB
dist/parse.weapp.js  766.99 kB │ gzip: 144.79 kB
dist/parse.weapp.min.js  287.35 kB │ gzip: 83.53 kB

@mtrezza
Copy link
Member

mtrezza commented Apr 14, 2025

In the future it should allow for tree shaking,

Let's merge this as perf: Optimize bundle packaging with vite, since it seems we don't have any specific improvement to tell for now. It will create a release.

@dplewis dplewis changed the title feat: Replace Browserify with Vite for bundling perf: Optimize bundle packaging with Vite Apr 14, 2025
@dplewis
Copy link
Member Author

dplewis commented Apr 14, 2025

@mtrezza Good idea! I change the title

mtrezza
mtrezza previously approved these changes Apr 14, 2025
@dplewis
Copy link
Member Author

dplewis commented Apr 14, 2025

@mtrezza I accidentally removed the legacy export. I added a test case and opened a issue #2564

This should be good now.

@dplewis dplewis requested a review from mtrezza April 14, 2025 18:02
@mtrezza
Copy link
Member

mtrezza commented Apr 15, 2025

I see you've made a commit after your last comment, is this now good to merge?

@dplewis
Copy link
Member Author

dplewis commented Apr 15, 2025

It is good to merge

@mtrezza mtrezza merged commit a4b19e5 into parse-community:alpha Apr 15, 2025
12 checks passed
parseplatformorg pushed a commit that referenced this pull request Apr 15, 2025
# [6.2.0-alpha.3](6.2.0-alpha.2...6.2.0-alpha.3) (2025-04-15)

### Performance Improvements

* Optimize bundle packaging with Vite ([#2553](#2553)) ([a4b19e5](a4b19e5))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.2.0-alpha.3

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Apr 15, 2025
@dplewis dplewis deleted the remove-browserify branch April 15, 2025 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released-alpha Released as alpha version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace browserify with webpack
4 participants