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

Add some new scripts to package.json, to make npm run command easier #8294

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

Conversation

pmario
Copy link
Member

@pmario pmario commented Jun 26, 2024

add some new scripts to package.json, to make npm run command easier

Copy link

vercel bot commented Jun 26, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
tiddlywiki5 ✅ Ready (Inspect) Visit Preview Jun 27, 2024 6:28pm

@pmario pmario changed the title add some new scripts to package.json, to make npm run command easier Add some new scripts to package.json, to make npm run command easier Jun 26, 2024
Copy link
Member

@Jermolene Jermolene left a comment

Choose a reason for hiding this comment

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

Thanks @pmario I am very much in favour of improving the core package.json scripts.

package.json Outdated Show resolved Hide resolved
@pmario pmario marked this pull request as draft June 27, 2024 10:50
Copy link
Member

@Jermolene Jermolene left a comment

Choose a reason for hiding this comment

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

Thanks @pmario these are powerful extensions, but do make the package.json file harder to read overall. I suppose that means that we'll need very good documentation here

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@pmario
Copy link
Member Author

pmario commented Jun 27, 2024

The structure changed completely.

  • We do have the basic commands:
    • start, test
  • A wiki command, that is used with +plugins/tiddlywiki/filesystem +plugins/tiddlywiki/tiddlyweb plugin prefixes
  • A cs command for "client server"
  • server and server-external-js, because there "naming pattern" is completely different

pluginlibrary is missing, because we need to handle the PR: Make pluginlibrary edition consistent with all other editions #8265 first

multiwikiserver is missing since it does not have a tiddlywiki.info file.

share has no tiddlers/ directory so it's missing too - I don't know what to do with it


If I clone a new repo, one of the first things I usually do is run npm run to see what's going on.
The command creates this output. Which should be somewhat self explaining.

image

Since wiki:info is the first thing, we do get some help info, without running any commands. So for most users that will be enough info to get going. If they run npm run wiki they get an error -> which is OK

image

Everything needed is there now, to be selected with the mouse and "copy / pasted" into the command line.

A similar output is created for npm run cs, which can be used for editions that have a separated *-server edition.

@Jermolene -- what do you think?

@pmario
Copy link
Member Author

pmario commented Jun 27, 2024

Feedback from npm run cs

image

@Jermolene
Copy link
Member

  • A cs command for "client server"

I think that is too obscure. Can we use "server" instead?

  • server and server-external-js, because there "naming pattern" is completely different

If we cater for external-js then perhaps we should also cater for lazy loading?

multiwikiserver is missing since it does not have a tiddlywiki.info file.

I think you've got a blank folder there because at some point in the past you've checked out the multi-wiki-support branch, and then weirdly when you check out a different branch, git seems not to remove the directory.

share has no tiddlers/ directory so it's missing too - I don't know what to do with it

That edition is just a wrapper around the "share" plugin, and doesn't need any tiddlers.

@pmario
Copy link
Member Author

pmario commented Aug 20, 2024

Just an info: Problems with missing tiddlywiki.info files has been solved with Fix tiddlywiki --editions command #8535

@pmario
Copy link
Member Author

pmario commented Aug 20, 2024

  • The cs command has been renamed to edition
  • The library command starts the local library server at port: 8888,
    • so it can be tested with a second server at port: 8080

npm run now shows this info

image

Possible commands are

  • npm start ... starts tw5.com-server --listen
  • npm test ... runs all tests

  • npm run wiki --edition=<wiki name> ... this one starts wikis without a server edition
  • npm run edition --edition=<edition name> ... this one starts wikis, that have a *-server edition
  • npm run library ... builds all plugins and starts the library server on port:8888
  • npm run server-external-js ... starts the corresponding edition
  • npm run help ... runs node ./tiddlywiki.js --help ... to get all CLI info

@pmario
Copy link
Member Author

pmario commented Aug 20, 2024

Updated screenshot in last post. Moved help, lint and lint:fix up in the hierarchy

@pmario
Copy link
Member Author

pmario commented Aug 20, 2024

I wrote:

  • server and server-external-js, because there "naming pattern" is completely different

Jeremy wrote:
If we cater for external-js then perhaps we should also cater for lazy loading?

I do have no idea how lazy loading works and how to set it up. @linonetwo can you make suggestions here?

@linonetwo ... In general, what do you think?

@linonetwo
Copy link
Contributor

Sure, I use lazy-loading everyday. Simply add a rootTiddler will enable lazy loading

https://github.com/tiddly-gittly/TidGi-Desktop/blob/fe3a29eff1fb7e70307c2f3d29483681d535403d/src/services/wiki/wikiWorker/startNodeJSWiki.ts#L125

Try add

"server:lazy": "node ./tiddlywiki.js ./editions/server root-tiddler=$:/core/save/lazy-all --listen",

or this, so there are existing tiddlers:

"server:lazy": "node ./tiddlywiki.js ./editions/tw5.com-server --listen root-tiddler=$:/core/save/lazy-all",

@pmario pmario marked this pull request as ready for review August 22, 2024 11:21
@pmario
Copy link
Member Author

pmario commented Aug 22, 2024

@Jermolene @linonetwo -- this one should be ready for review now

@linonetwo
Copy link
Contributor

linonetwo commented Aug 22, 2024

How about add default script like

"wiki:default": "node ./tiddlywiki.js +plugins/tiddlywiki/filesystem +plugins/tiddlywiki/tiddlyweb ./editions/dev --listen",

so we can click on button to open, instead of use cli. I usually start/restart wiki in this way:

图片

or hover here

图片

only need to click on it.

Anyway, a clickable "start" is enough for me to test my PRs.

@pmario
Copy link
Member Author

pmario commented Aug 22, 2024

IMO for debugging you can use F5 and define as many configs as you need there. npm scripts imo are for the CLI

image

@Jermolene
Copy link
Member

It occurs to me that the use of npm run and the details of the supported commands should all be documented on tiddlywiki.com.

@pmario
Copy link
Member Author

pmario commented Aug 22, 2024

I can do this, once we are OK with the names and the mechanisms that are outlined here. The minimum info needed is at: #8294 (comment) atm.

@pmario
Copy link
Member Author

pmario commented Aug 23, 2024

@Jermolene ... I think this doc should be in the dev-edtion. right?

@saqimtiaz
Copy link
Member

One of my mainstays is an npm run dev script that starts node.js with the --watch-path flag so that any changes to a core file automatically restart the server. Something similar in the bundled scripts would be very welcome.

See https://nodejs.org/api/cli.html#--watch-path

@pmario
Copy link
Member Author

pmario commented Aug 23, 2024

@saqimtiaz ... This will require Node.js 22.x as a minimum version, if I can see that right. Latest LTS is 20.x
@Jermolene ... Do we want that

I personally would like to have a native -watch feature

@saqimtiaz
Copy link
Member

the --watch-path argument was added in node v18.11.10 as an experimental feature and is stable in node v20.x LTS.

@Jermolene
Copy link
Member

@Jermolene ... I think this doc should be in the dev-edtion. right?

I think it makes sense to document them along with the other command line details in tw5.com.

@Jermolene
Copy link
Member

In terms of which Node.js versions that we target, perhaps it would make sense to adopt the policy to target the oldest LTS version of Node.js, which would currently be Node.js 18. That's the version used by our CI tests, as it happens.

We could still have an npm script that uses --watch-path, though, since the scripts are so loosely coupled with the core. It would perhaps give people a reason to upgrade.

@pmario
Copy link
Member Author

pmario commented Aug 26, 2024

I think we should target the oldest active LTS version which would be Node.js 20.x.

The maintenance time for 18.x ends in April 2025.. If we start to switch version at that date we have no time left to deal with any problems in the next version.

Schedule see: https://nodejs.org/en/about/previous-releases#release-schedule

@pmario
Copy link
Member Author

pmario commented Aug 27, 2024

@Jermolene @saqimtiaz ... I did add npm run watch which starts the tw5.com-server edition.

The problem I see is, that the client is not notified, that there is a new server running. But the command seems to work

@pmario
Copy link
Member Author

pmario commented Sep 9, 2024

@Jermolene -- IMO this one can be merged for further testing. -- It only will affect developers -- So users should have no problems

@pmario pmario marked this pull request as draft September 9, 2024 13:53
@pmario
Copy link
Member Author

pmario commented Sep 9, 2024

Converted to draft -- It does not work for me anymore.

@pmario pmario marked this pull request as ready for review September 9, 2024 13:56
@pmario
Copy link
Member Author

pmario commented Sep 9, 2024

Sorry had a typo in the command line :/ Everything is OK

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.

4 participants