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

chore(dev): migrate away from parcel and replace with browserify #3658

Merged
merged 3 commits into from
Jun 28, 2021

Conversation

jsjoeio
Copy link
Contributor

@jsjoeio jsjoeio commented Jun 23, 2021

This PR migrates us away from Parcel by using tsc and browserify to accomplish the same thing.

Originally, we needed to upgrade Parcel from v1 to v2 due to maintainers deprecating v1 and a bunch of security vulnerabilities that we couldn't patch in v1. We were going to use v2 but it has breaking changes. So we decided to replace it with browserify which is a bit more lightweight and does the same thing.

Changes

  • remove parcel
  • use tsc to compile .ts files and browserify to bundle a few files used in the browser
  • reference css files directly in HTML instead of including with js bundles
  • remove minification of browser files (issues with es2015+ support using browserify plugin tinyify)

Screenshot

Screenshot after building and running locally
Screen Shot 2021-06-10 at 2 23 28 PM

Checklist

  • updated CHANGELOG.md

Related

@jsjoeio jsjoeio changed the title jsjoeio/migrate-parcel chore(dev): migrate away from parcel and replace with browserify Jun 23, 2021
@codecov
Copy link

codecov bot commented Jun 23, 2021

Codecov Report

Merging #3658 (0688ff8) into main (df01808) will decrease coverage by 1.40%.
The diff coverage is 100.00%.

❗ Current head 0688ff8 differs from pull request most recent head 6e1455d. Consider uploading reports for the commit 6e1455d to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3658      +/-   ##
==========================================
- Coverage   61.28%   59.87%   -1.41%     
==========================================
  Files          35       35              
  Lines        1803     1797       -6     
  Branches      409      364      -45     
==========================================
- Hits         1105     1076      -29     
- Misses        556      605      +49     
+ Partials      142      116      -26     
Impacted Files Coverage Δ
src/browser/register.ts 100.00% <ø> (ø)
src/browser/pages/login.ts 100.00% <100.00%> (+100.00%) ⬆️
src/browser/pages/vscode.ts 60.60% <100.00%> (+1.23%) ⬆️
src/node/wrapper.ts 22.07% <0.00%> (-11.05%) ⬇️
src/node/vscode.ts 19.48% <0.00%> (-8.02%) ⬇️
src/node/plugin.ts 64.60% <0.00%> (-3.59%) ⬇️
src/node/routes/login.ts 40.74% <0.00%> (-1.86%) ⬇️
src/node/app.ts 69.04% <0.00%> (-1.69%) ⬇️
src/node/util.ts 69.27% <0.00%> (-1.68%) ⬇️
src/node/http.ts 34.92% <0.00%> (-1.15%) ⬇️
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df01808...6e1455d. Read the comment docs.

@jsjoeio jsjoeio self-assigned this Jun 23, 2021
@jsjoeio jsjoeio added the chore Related to maintenance or clean up label Jun 23, 2021
@jsjoeio jsjoeio added this to the 3.11.0 milestone Jun 23, 2021
@jsjoeio
Copy link
Contributor Author

jsjoeio commented Jun 23, 2021

Merging #3658 (decd057) into main (fbba8e8) will decrease coverage by 1.47%.
The diff coverage is 33.33%.

Note, I'm not sure if this is 100% accurate. I did indeed add tests. However, half of the files under "Impacted Files" I didn't actually touch. I tried to investigate further the codecov docs and tried a couple things to fix this but didn't have any luck.

@jsjoeio jsjoeio marked this pull request as ready for review June 23, 2021 18:01
@jsjoeio jsjoeio requested a review from a team as a code owner June 23, 2021 18:01
test/browser/pages/login.test.ts Outdated Show resolved Hide resolved
test/browser/pages/login.test.ts Outdated Show resolved Hide resolved
@jsjoeio jsjoeio requested a review from code-asher June 28, 2021 17:47
@jsjoeio jsjoeio force-pushed the jsjoeio/migrate-parcel branch 2 times, most recently from 526e69a to 4383911 Compare June 28, 2021 18:19
This also refactors a couple CSS stylesheets to be referenced directly in the
HTML files.

And it removes any CSS imports from src/browser files.
@jsjoeio jsjoeio merged commit e1d291e into main Jun 28, 2021
@jsjoeio jsjoeio deleted the jsjoeio/migrate-parcel branch June 28, 2021 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Related to maintenance or clean up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants