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

Bump node to 18 #57

Merged
merged 4 commits into from
May 17, 2024
Merged

Bump node to 18 #57

merged 4 commits into from
May 17, 2024

Conversation

HarlemSquirrel
Copy link
Contributor

No description provided.

@@ -30,7 +30,8 @@
"settings": {
"import/parsers": {
"@typescript-eslint/parser": [".ts", ".tsx"]
}
},
"jest": { "version": 27 }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need this and eslint-jest-plugin because eslint-config-preact requires it 😑
preactjs/eslint-config-preact#19 (comment)

@HarlemSquirrel
Copy link
Contributor Author

Was able to build and install locally but so far unable to connect to KeeWeb from extension when built from this branch.

@HarlemSquirrel
Copy link
Contributor Author

Well for some reason tonight when I rebuilt it now it's working 😅

@HarlemSquirrel HarlemSquirrel requested a review from Aetherinox May 9, 2024 03:41
@HarlemSquirrel HarlemSquirrel marked this pull request as ready for review May 9, 2024 03:41
@Aetherinox
Copy link

Aetherinox commented May 9, 2024

Got to love when that happens.
FYI, as of today's changes, ensure your environment uses only one of the following NodeJS versions:

  • ^18.12.0
  • ^20.9.0
  • >=22.0.0

If KeeWeb is built on any other version; there will be dependency errors and things will break. I'm running on v22 and focusing the builds around that for the simple fact of v22 getting require()ing ES modules. But NodejS LTS 20 works just as well.

Is this PR completely done now?

@HarlemSquirrel
Copy link
Contributor Author

I'd like to test the chrome extension but haven't yet. That's it

@HarlemSquirrel
Copy link
Contributor Author

Having trouble connecting to Chrome with local built extension. I think it may be due to the extension ID being different but updating https://github.com/keeweb/keeweb/blob/master/desktop/scripts/const/extension-ids.js and running keeweb locally doesn't solve it for me.

@HarlemSquirrel
Copy link
Contributor Author

Still not working with node 16 or 18 on master branch, last tag, or this branch. I'm clearly missing something.

@HarlemSquirrel
Copy link
Contributor Author

@antelle sorry to bug you but do you have any thoughts on what I might be missing here running chrome/chromium extension locally and connecting to keeweb desktop? Firefox works but in Chromium I get no logs in the desktop console which leads me to believe it's not receiving any messages at all. The background extension console says KeeWeb host disconnected Specified native messaging host not found.

@antelle
Copy link
Member

antelle commented May 16, 2024

Do you have the messaging host config enabled? When you check the checkbox in settings in KeeWeb, it creates the config, but I assume it has a wrong extension ID there

@HarlemSquirrel
Copy link
Contributor Author

OK I'm not sure why but I got it working on a different computer running Pop OS 22.04 so I'm going to say this is working OK with Firefox and Chrome.

@HarlemSquirrel
Copy link
Contributor Author

I also successfully tested edge

@HarlemSquirrel HarlemSquirrel merged commit 471cd3b into keeweb:master May 17, 2024
5 checks passed
@HarlemSquirrel HarlemSquirrel deleted the node-18 branch May 17, 2024 20:15
@Aetherinox
Copy link

Aetherinox commented May 19, 2024

Did you get all of it working now? I still need to mess with this to get a feel for it. But I'll have to do it after I finish up the stuff with KeeWeb. Don't want to start flip-flopping.

You can merge this whenever you feel it's fully done, you've got more experience with this aspect, so I'll trust your judgement. I just wanted to make sure we start getting packages updated so we can work with the latest.

@HarlemSquirrel
Copy link
Contributor Author

Yeah it's working locally. There does appear to be a validation issue when trying to publish the Firefox addon but I have a separate PR for that

@Aetherinox
Copy link

Aetherinox commented May 19, 2024

NIce, progress. I'll definitely look into these plugins after I wrap up my end. I spend half my time reading pages of code so that I know what all is done in that segment, trying to get myself familiarized with this app like I would my own. Mainly prepping myself for the Electron v14+ migration since a lot has changed, and I don't want ti keep us running on v13 for too long.

I've looked at the future versions, and once we make ourselves compat with 14, we can easily bump up to v20 before there's any other major changes to do. Just minor API changes in-between.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants