Skip to content
This repository has been archived by the owner on Apr 28, 2024. It is now read-only.

Support shorthand version in Node.js #15

Merged
merged 3 commits into from
Feb 21, 2024
Merged

Conversation

bytemain
Copy link
Member

@bytemain bytemain commented Feb 21, 2024

Support install node by nodejs@20 expression, also support nodejs@x.x

CleanShot 2024-02-21 at 12 13 37@2x

CleanShot 2024-02-21 at 12 13 55@2x

Fix #14

@bytemain
Copy link
Member Author

the nodejs.lua and npmmirror.lua has many duplicate code, so I think that if a plugin can contain multiple files, it will be much better

@bytemain
Copy link
Member Author

or we can add a build stage, compile all lua scripts into a single lua script

nodejs/nodejs.lua Outdated Show resolved Hide resolved
nodejs/nodejs.lua Show resolved Hide resolved
nodejs/npmmirror.lua Show resolved Hide resolved
nodejs/nodejs.lua Outdated Show resolved Hide resolved
@aooohan
Copy link
Member

aooohan commented Feb 21, 2024

the nodejs.lua and npmmirror.lua has many duplicate code, so I think that if a plugin can contain multiple files, it will be much better

Yes, this is better. I'm also thinking about this, currently the plugin is distributed as http, which is more friendly and simply for a single file, and I'm thinking about changing to git that we can contain multiple files and also utilize some existing lua library files.

@bytemain
Copy link
Member Author

but I found another problem, the use command do not support this feature:
CleanShot 2024-02-21 at 15 31 18@2x

and it seem use command do not have a lua hook

@aooohan
Copy link
Member

aooohan commented Feb 21, 2024

but I found another problem, the use command do not support this feature:

@bytemain Because different SDKs may have different habits, when switching, it is actually determined based on version field returned by PLUGIN:PreInstall.

https://github.com/bytemain/version-fox-plugins/blob/efe77dd16ec43fa38f97086e44e0ebfb4c9d1e43/nodejs/nodejs.lua#L62-L66

In the current case, even though you're passing 20, after parsing it through PLUGIN:PreInstall, you're still returning the specific version. But I wouldn't recommend it at the moment.

Maybe it will be better to have an alias function in the future. ;)

Copy link
Member

@aooohan aooohan left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for your first contribution!

@aooohan aooohan merged commit def6edf into version-fox:main Feb 21, 2024
@aooohan
Copy link
Member

aooohan commented Feb 21, 2024

It's available now!
image

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

Successfully merging this pull request may close these issues.

Support nodejs@20 expression
2 participants