polar icon indicating copy to clipboard operation
polar copied to clipboard

Bug: M1 Mac: yarn dev only works with node 14 and not node 17

Open amovfx opened this issue 1 year ago • 11 comments

Describe the bug yarn dev wont work on node 17, users need to be on node 14 Maybe a line could be added to the Pre req in the Contributing.md?

I easily switched my node version to 14 so no big deal.

To Reproduce Steps to reproduce the behavior:

  1. Install node v17
  2. install polar dips
  3. yarn dev
  4. See error
Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './lib/tokenize' is not defined by "exports" in /Users/andrew/git/polar/node_modules/postcss-safe-parser/node_modules/postcss/package.json

Expected behavior Note in the Contributing.md about node version on M1

Screenshots If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: MacOS Ventura 13.0
  • Polar c6ab9f70
  • Docker version 20.10.21 Docker Compose version 2.12.2

Additional context Mac M1

amovfx avatar Nov 14 '22 22:11 amovfx

Thanks for reporting this problem. I'm using Node v16.16.0 and it works, but it indeed breaks when using v17 or v18. I'll work on fixing this soon.

Just as a note, It's not recommended to use odd versions of Node because the devs do not offer long term support of odd versions.

jamaljsr avatar Nov 14 '22 23:11 jamaljsr

Thanks for the heads up.

amovfx avatar Nov 15 '22 22:11 amovfx

I'm going to leave this open until the problem is fixed.

jamaljsr avatar Nov 16 '22 23:11 jamaljsr

Hi @jamaljsr, I still owe you one answer relating to a line I added to the TODOs list which was related to upgrading project dependencies. I was referring to deps like Electron.js (we're currently on v13 but the current stable release is V21) and perhaps React (we use v17 vs current v18).

Don't get me wrong, I'm not in favor of chasing the latest and greatest dependencies and shiny new libs, however, technical debts do tend to accumulate in projects over time, and the longer we wait the harder it gets to catch up.

Specifically relating to Electron.js, I believe that in one of the releases, they made substantial security changes in the working model, creating almost total separation between the backend node.js process and the front end, using a preload script injected into the browser used for ipc comms between the processes with total isolation, so the frontend webapp never requires/imports any node.js or electron.js dependencies directly.

I'm certain you know all that already, just wanted to touch on it b/c I think I did not get to answer your question previously :-)

zackypick avatar Nov 18 '22 05:11 zackypick

Thanks for the clarification. I agree with everything you mention. I'd like to upgrade Electron and React to the latest versions. I've been neglecting them because I know those are pretty big projects that will take a bunch of time and provide very little impact to the UX of Polar. I've essentially been waiting for something to break and box me into a corner where I have no choice but to do the work and upgrade them. If there's anyone out there ;) that would want to work on this sooner, I'd greatly appreciate it.

jamaljsr avatar Nov 18 '22 21:11 jamaljsr

Thanks for the clarification. I agree with everything you mention. I'd like to upgrade Electron and React to the latest versions. I've been neglecting them because I know those are pretty big projects that will take a bunch of time and provide very little impact to the UX of Polar. I've essentially been waiting for something to break and box me into a corner where I have no choice but to do the work and upgrade them. If there's anyone out there ;) that would want to work on this sooner, I'd greatly appreciate it.

Right, that is a scary migration, did a few migrations over time and fun was not part of the jargon I'd use to describe it :-)

I'd gladly look into that too sometime, but that would certainly break a whole lot of things, especially the Electron one.

Having said that, what really interests me most (and what brought me to this project in the first place) was to be able and work on bitcoin & LN code, to get closer to the protocols, so I could start shifting towards contributing to something I really care and believe in for the future.

zackypick avatar Nov 19 '22 07:11 zackypick

Based on previous such migrations, here are some of the major steps that would be needed (not necessarily in that order):

  1. New features/releases freeze (or at least slowed down significantly) until migration is done
  2. Clean the UI from any Node.js (e.g. Node.Timer we happened to touch on) and Electron imports (code and types included)
  3. Migrate to latest React stable version
  4. Revisit/rewrite the ipc mechanism (sync/async messages front<-->back)
  5. Newest stable version of Electron.js
  6. Possibly introduce a monorepo (e.g. Nx), resulting in 3 projects/libs at bare minimum (a) pure React frontend (b) pure Electron/node backend (c) shared lib(s)
  7. Handle Electron windowing issues and glitches across OSs (for instance there is now the dark mode Win hack, with new versions of Electron it is very much likely similar hacks would be required)
  8. Fix/rewrite/new unit and E2E tests
  9. Auto-updater revisit (not sure we have autoupdater working today?)
  10. Installer will probably be needed to be rewritten too, potentially using other libs
  11. Signing the executables (again, not sure we have that today?) - this is a big one too

That is a big effort. I wish I had the time to do it, but I have my fiat job paying my bills (I'm a contractor) and also I need to take care of my devs team to have work as well, so currently not looking practical (and would take too much time doing that on weekends etc.)

But who knows? maybe it would become important for Lightning Labs or another company and they could sponsor the effort someday.

Anyway that is the rough breakdown of how I see this migration happening from my experience, thought it might be worth to have that written down for the future :-)

zackypick avatar Nov 19 '22 08:11 zackypick

Thanks for the write-up. You've listed pretty much everything that I believe would be necessary to perform the upgrade. It is indeed a lot of work. I will eventually get to it. Appreciate the discussion.

jamaljsr avatar Nov 19 '22 21:11 jamaljsr

Sure thing! when you get around to it maybe you'd like to let me know, and perhaps we could then make it a concerted effort.

zackypick avatar Nov 20 '22 04:11 zackypick

BTW I see my automining PR is not listed for some reason. Should I open a new one instead once I get the compilation errors resolved?

zackypick avatar Nov 20 '22 04:11 zackypick

It looks like you accidentally closed the PR https://github.com/jamaljsr/polar/pull/613. You can reopen it and continue working on it there.

jamaljsr avatar Nov 20 '22 12:11 jamaljsr

Fixed in #827

jamaljsr avatar May 03 '24 13:05 jamaljsr