-
Notifications
You must be signed in to change notification settings - Fork 332
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
Replace webpack with vite.js #364
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: 5afac23 The changes in this PR will be included in the next version bump. This PR includes changesets to release 40 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Screenshots: ✅
There are no changes in the screenshots for this PR. If this is expected, you are good to go. |
Codecov Report
@@ Coverage Diff @@
## develop #364 +/- ##
===========================================
- Coverage 47.94% 47.60% -0.34%
===========================================
Files 602 598 -4
Lines 26550 26419 -131
Branches 6792 6760 -32
===========================================
- Hits 12730 12578 -152
- Misses 13768 13790 +22
+ Partials 52 51 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm crazy pr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, works well on LLM and LLD starts so fast, I like it ✌️ thx for the work on that
📝 Description
This PR aims to replace
webpack
with vite.js to improve the developer experience while working on ledger live desktop.Benefits: fast cold boot time, instant hot reload ✨ .
Unfortunately, vite.js is quite rigid, and the following impactful changes have been made to make it work properly:
.js
to.jsx
files when needed (jsx inside).@ledgerhq/live-common
to esm in addition to commonjs and added subpath exports.@ledgerhq/hw-app-btc
to esm in addition to commonjs and added subpath exports.@ledgerhq/devices
./lib
prefixed paths to@ledgerhq/live-common
and@ledgerhq/hw-app-btc
.🔥 There are breaking changes for
@ledgerhq/live-common
,@ledgerhq/devices
and@ledgerhq/hw-app-btc
consumersReason
To fully embrace the "bundleless"
vite.js
approach, it is necessary to transpile our packages contained in the monorepository to the ESM format.At the same time we cannot afford to break commonjs support for node.js code requiring the same packages, so we must also transpile to commonjs.
The hard part is that we have shared code (renderer/main processes) in LLD and depending on which entry point we must be able to dispatch to the right format.
Until now we added the
/lib
prefix to directly point to transpiled files when importing the library (ex:import whatever from "@ledgerhq/live-common/lib/whatever"
). This is not possible anymore since we have both/lib
and/lib-es
directories.The changes
The "official" workaround is to add subpath exports to silently map subpaths to
/lib/*
and/lib-es
.The configuration in the
package.json
file is something like:Which makes the following import…
import { createDataModel } from "@ledgerhq/live-common/DataModel";
…mapped to…
import { createDataModel } from "@ledgerhq/live-common/lib-es/DataModel";
…and the following require…
const { createDataModel } = require("@ledgerhq/live-common/DataModel");
…is mapped to:
const { createDataModel } = require("@ledgerhq/live-common/lib/DataModel");
The limitations
As highlighted here (nodejs/node#39994), it is not possible to target folders directly when using subpath exports.
The workaround is to suffix the call with
/index
(or/
).For instance,
import * as currencies from "@ledgerhq/live-common/currencies";
must be rewritten toimport * as currencies from "@ledgerhq/live-common/currencies/index";
This PR takes care of renaming every imports properly but will break for external projects importing subpaths with
/lib
and targeting a folder.❓ Context
✅ Checklist
@ledgerhq/live-common
@ledgerhq/hw-app-btc
@ledgerhq/devices
📸 Demo
Full boot + hot reload ⚡
screen-recording-2022-06-17-at-124531_8i2MaEI4.mp4
Just hot reload
Screen.Recording.2022-06-14.at.15.10.36.mov
🚀 Expectations to reach
Please make sure you follow these Important Steps.
Pull Requests must pass the CI and be internally validated in order to be merged.