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

Update packages, eslint, and fix missing packages. #111

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

stroiman
Copy link

@stroiman stroiman commented Aug 2, 2024

This update outdated packages, updates eslint, and adds a few missing packages

Fix missing packages

Build tools, like eslint, should be added as dev dependencies to JS projects. Then it's the concrete version in your project used when building; rather than depending on an uncontrollable context; which plugins developers or build servers have installed globally.

Update eslint

The .eslintrc format is obsolete. It has been updated to the new flat file format

Update packages

Many packages were very old; including TypeScript that was at 4.x.

Eslint is only updated to version 8; rather than 9 - because the typescript eslint module doesn't support eslint@8 as a peer dependency; making npm fail during install.

@types/node should be of the same major version as the actual node.js runtime. In my local Obsidian developer console, process.versions reveals that it uses version 18 (not 20?), which is why I updated that module to version 18.

@joethei
Copy link
Contributor

joethei commented Aug 2, 2024

The oldest node version that a user can run, and still have a supported installer version is 16.13.2
You also have listed @types/node twice

And when using typescript-eslint one should not use the JS presets, since they overlap somewhat.

@stroiman
Copy link
Author

stroiman commented Aug 5, 2024

Allright, I rolled back the @types/node update

But it is correct that Obsidian comes bundled with a version of node.js? Is there an overview which Obsidian versions use which node versions? E.g. if I decided to use a feature that only exists in node 18, I should make sure that my minAppVersion is set accordingly. (hypothetical right now - not something I have any plans of)

I'll take a look at the eslint settings later today, but I didn't write the configuration; I just used the migration tool to migrate the existing config from the original tempalte.

> npx @eslint/migrate-config .eslintrc.json

As documented here: https://eslint.org/docs/latest/use/configure/migration-guide

That also mentioned that I needed the other eslint packages, that I added.

@joethei
Copy link
Contributor

joethei commented Aug 5, 2024

The Obsidian installer comes bundled with Electron & Node, among other things.
The app version is seperate from that, and thus minAppVersion can't be used to make sure a specific Node.js version is installed.

See also: https://help.obsidian.md/Getting+started/Update+Obsidian#Installer+updates

This updates the eslint configuration to the new flat file format, which
is the future format.

Also adds eslint as a dev dependency. JS Projects should have locally
installed the correct versions of development tools like
eslint/prettier, etc. So developer experience is consistent; and not
depending on external context, like globally installed versions; but the
version that matches the configuration.

Also updates outdated packages, like typescript

Would have liked to update to eslint@9, but the typescript module does
not yet support 9 as a peer dependency.
This replaces the older @typescript-eslint/* packages with new flat-file
aware configuration.
@stroiman
Copy link
Author

stroiman commented Aug 5, 2024

Allright, a bit of searching made me understand the steps in the auto-converted file does; as well that there's a new typescript plugin compatible with the flat format, simplifying the configuration; and removing the necessity for some of the added packages.

Seems to work, i.e. linter errors are generated when I deliberately create them; both in my editor; and from command line. I'll take the modified config for a spin in my WIP plugin project.

Btw, maybe it would make sense to include a 'lint' step in the 'build' script?

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.

2 participants