Skip to content

chore: migrate from lerna bootstrap to npm workspaces#2094

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.

Sign up for GitHub

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

Merged
trusktr merged 8 commits into from
Jun 24, 2023
Merged

chore: migrate from lerna bootstrap to npm workspaces #2094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
293d86f
chore: migrate from lerna bootstrap to npm workspaces
JamesHenry Jun 16, 2023
77a2186
chore: add missing npm install to readme
JamesHenry Jun 21, 2023
ebc36ab
Merge branch 'develop' into develop
sy-records Jun 21, 2023
a285bfd
chore: update readme
JamesHenry Jun 21, 2023
71ece72
Merge branch 'develop' into develop
sy-records Jun 21, 2023
1f95752
docs: change readme
sy-records Jun 23, 2023
6f9aa1a
chore: ignore package-lock.json for docsify-server-renderer
sy-records Jun 23, 2023
20896cf
Merge branch 'develop' into develop
trusktr Jun 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -26,10 +26,6 @@ jobs:
- name: Lint
run: npm run lint

- name: Verify dependencies [server-renderer]

@JamesHenryJamesHenryJun 16, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This is no longer needed, npm ci natively understands workspaces and will install everything from the root

@Koooooo-7Koooooo-7Jun 17, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is no longer needed, npm ci natively understands workspaces and will install everything from the root

Hi James, we actually have 2 repos and the renderer one is the subfolder of root but not a module. So we need run ci seperated for them.

@trusktrtrusktrJun 24, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey @Koooooo-7, can you describe the issue here more?

What James is mentioning is that the above step,

 run: npm ci --ignore-scripts

also installs dependencies for server-renderer.

@trusktrtrusktrJun 24, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we're fine: I don't think many people (if any?) are using SSR, and our current dev:ssr script is even broken like so:

$npmrun dev:ssr/Users/trusktr/src/docsifyjs+docsify/packages/docsify-server-renderer/build.js:8varfetch=_interopDefault(require('node-fetch'));^Error[ERR_REQUIRE_ESM]: require()ofESModule/Users/trusktr/src/docsifyjs+docsify/packages/docsify-server-renderer/node_modules/node-fetch/src/index.jsfrom/Users/trusktr/src/docsifyjs+docsify/packages/docsify-server-renderer/build.jsnotsupported.Insteadchangetherequireofindex.jsin/Users/trusktr/src/lume+lume+develop/packages/docsifyjs+docsify/packages/docsify-server-renderer/build.jstoadynamicimport()whichisavailableinallCommonJSmodules.atObject.<anonymous>(/Users/trusktr/src/lume+lume+develop/packages/docsifyjs+docsify/packages/docsify-server-renderer/build.js:8:29)atObject.<anonymous>(/Users/trusktr/src/lume+lume+develop/packages/docsifyjs+docsify/server.js:6:20){code: 'ERR_REQUIRE_ESM'}Node.jsv18.13.0lib/plugins/emoji.js
ERROR: "serve:ssr"exitedwith1.

I a new issue which should address this: #2102

I think it would be ok to merge this then fix any issues we may find, if we even care about them.

Even better: I recommend we just delete the SSR stuff because it is a pain to maintain that none of us have time for. In fact, install Docsify plugins and SSR breaks very easily. It is in a very alpha quality that we aren't using ourselves and honestly haven't cared for, so I think deleting SSR, and improving our code base (move to ESM, clean up some logic), would help us better prepare for thinking about the proper way to implement SSR.

If we decide to then restore SSR, I think we should convert to a Docsify component (f.e. using Solid.js, React, Vue, Svelte, or etc) and let a tool like Astro (maintained by a team of people who think about SSR and SSG all day every day, and are making the latest advancements in the area) give us the best SSR and SSG setup that we can simply ride upon. This will be very powerful, and without us having our own SSR implementation that doesn't work well and is far from complete.

With this in mind, I would be ok to roll with this, as I tested non-SSR Docsify and the dev experience is working great.

working-directory: ./packages/docsify-server-renderer
run: npm ci --ignore-scripts

test-jest:
runs-on: ${{ matrix.os }}
strategy:
Expand Down
10 changes: 4 additions & 6 deletions README.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -76,6 +76,10 @@ You can use Gitpod (a free online VS Code-like IDE) for contributing. With a sin
- install the dependencies.
- start `npm run dev`.

```bash
npm install && npm run dev
```

So that you can start straight away.

[![Open in Gitpod](https://gitpod.io/button/open-in-gitpod.svg)](https://gitpod.io/#https://github.com/docsifyjs/docsify)
Expand All@@ -86,12 +90,6 @@ So that you can start straight away.
- Push to the branch: `git push origin my-new-feature`
- Submit a pull request

## Development

```bash
npm run bootstrap && npm run dev
Comment thread
sy-records marked this conversation as resolved.
```

## Backers

Thank you to all our backers! 🙏 [[Become a backer](https://opencollective.com/docsify#backers)]
Expand Down
6 changes: 6 additions & 0 deletions docs/emoji.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -34,6 +34,8 @@ Below is a complete list of emoji shorthand codes. Docsify can be configured to

:accept: `:accept:`

:accessibility: `:accessibility:`

:accordion: `:accordion:`

:adhesive_bandage: `:adhesive_bandage:`
Expand DownExpand Up@@ -920,6 +922,8 @@ Below is a complete list of emoji shorthand codes. Docsify can be configured to

:department_store: `:department_store:`

:dependabot: `:dependabot:`

:derelict_house: `:derelict_house:`

:desert: `:desert:`
Expand DownExpand Up@@ -1238,6 +1242,8 @@ Below is a complete list of emoji shorthand codes. Docsify can be configured to

:fishing_pole_and_fish: `:fishing_pole_and_fish:`

:fishsticks: `:fishsticks:`

:fist: `:fist:`

:fist_left: `:fist_left:`
Expand Down
5 changes: 1 addition & 4 deletions lerna.json
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
{
"lerna": "2.0.0-rc.5",
Comment thread
trusktr marked this conversation as resolved.
"packages": [
Comment thread
trusktr marked this conversation as resolved.
"packages/*"
],
"$schema": "node_modules/lerna/schemas/lerna-schema.json",
Comment thread
trusktr marked this conversation as resolved.
"version": "0.0.0"
}
Loading