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

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

Closed
amovfx opened this issue Nov 14, 2022 · 12 comments
Closed

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

amovfx opened this issue Nov 14, 2022 · 12 comments
Labels
bug Something isn't working up for grabs Anyone can work on this

Comments

@amovfx
Copy link
Contributor

amovfx commented Nov 14, 2022

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 c6ab9f7
  • Docker version 20.10.21
    Docker Compose version 2.12.2

Additional context
Mac M1

@amovfx amovfx added the bug Something isn't working label Nov 14, 2022
@jamaljsr
Copy link
Owner

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.

@amovfx
Copy link
Contributor Author

amovfx commented Nov 15, 2022

Thanks for the heads up.

@amovfx amovfx closed this as completed Nov 15, 2022
@jamaljsr
Copy link
Owner

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

@jamaljsr jamaljsr reopened this Nov 16, 2022
@zackypick
Copy link
Contributor

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 :-)

@jamaljsr
Copy link
Owner

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.

@zackypick
Copy link
Contributor

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
Copy link
Contributor

zackypick commented Nov 19, 2022

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 :-)

@jamaljsr
Copy link
Owner

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.

@zackypick
Copy link
Contributor

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
Copy link
Contributor

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?

@jamaljsr
Copy link
Owner

It looks like you accidentally closed the PR #613. You can reopen it and continue working on it there.

@jamaljsr jamaljsr added the up for grabs Anyone can work on this label Dec 19, 2023
@jamaljsr
Copy link
Owner

jamaljsr commented May 3, 2024

Fixed in #827

@jamaljsr jamaljsr closed this as completed May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working up for grabs Anyone can work on this
Projects
None yet
Development

No branches or pull requests

3 participants