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

Behavior Change in Public Directory Path Resolution Since @slidev/cli@0.48.0-beta.4 #1401

Closed
Ynn opened this issue Mar 9, 2024 · 8 comments · Fixed by #1403
Closed

Behavior Change in Public Directory Path Resolution Since @slidev/cli@0.48.0-beta.4 #1401

Ynn opened this issue Mar 9, 2024 · 8 comments · Fixed by #1403
Assignees
Labels
help wanted Extra attention is needed

Comments

@Ynn
Copy link

Ynn commented Mar 9, 2024

I upgraded from @slidev/cli@0.48.0-beta.1 to @slidev/cli@0.48.0-beta.24 and noticed a change in behavior. So, I tested all versions between 1 and 24, and it seems that this change occurred starting from @slidev/cli@0.48.0-beta.4.

Before this change, I could have a slidev directory in which I installed everything, and a subdirectory called slides where I had my folders:

slidev
    |-- node_modules
    ... all slidev dependencies & themes
    `-- slides
        |`-- public/images
        | style.css
        | my-slide.md

My images and style.css file were found without any problems, and I could even have components in the slides folder that were also found.

Since version @slidev/cli@0.48.0-beta.4, the public folder is no longer mounted from slidev/slides/public but from slidev/public only. This means I have to move my files there.

This is not convenient for me because I find it much clearer to separate presentations into different folders while keeping the theme and slidev in the main directory.

I also tested version 0.47.5, and it works as I wanted.

I suspect this might be due to one of these changes:

Is this new behavior intended? What can I do to revert to the previous situation?

@KermanX
Copy link
Member

KermanX commented Mar 9, 2024

I think this is caused by #1308. In #1308, we assume that the project root is the closest folder to cwd with a package.json. To use slidev/slides/public as the public folder, you can add a package.json to slidev/slides.

(Before #1308, the project root (i.e. user root) was where the slides.md is)

@Ynn
Copy link
Author

Ynn commented Mar 9, 2024

Thank you for your quick reply :) . What should this minimal package.json contain. Does this mean that I'll have to "pollute" my slides directory with node_modules files on the side?

@KermanX
Copy link
Member

KermanX commented Mar 9, 2024

Thank you for your quick reply :) . What should this minimal package.json contain. Does this mean that I'll have to "pollute" my slides directory with node_modules files on the side?

I think { "name": "my-slides", "version": "0.0.0" } is enough.

@Ynn
Copy link
Author

Ynn commented Mar 9, 2024

Thanks for your help. That was what i first try but it does not seems to work. I have :

slidev
    |package.json
    |-- node_modules with all slidev dep & themes
    `-- slides
        |`-- public/images
        | style.css
        | my-slide.md
        | package.json

@KermanX
Copy link
Member

KermanX commented Mar 9, 2024

Could you share how you start Slidev dev server?

@Ynn
Copy link
Author

Ynn commented Mar 9, 2024

I was using :

npx slidev slides/slide.md --remote

but going inside the slide directory and doing :

npx slidev slide.md --remote

seems to work. I am not sure why. The only downside is that i have to have a package.json in my slide directory :)

For the context, i am trying to have all slidev env in a docker env : https://github.com/Ynn/slides.tools . Contrary to what tangramor /slidev_docker is doing i setup all the dependencies and modules inside the docker image to avoid having node_modules in my slides directory. I want to have something static as i don't want any surprise when editing or playing my slides.

I will try that and mark as fix if it works well. Would it be possible to have an option to change the root from the cli ? This way a package.json wouldn't be needed.

@KermanX
Copy link
Member

KermanX commented Mar 9, 2024

Thank you for pointing this out. I know what the problem is now. Currently, the user root is the closest folder to cwd
with a package.json, which may be not the best solution...

I think there are 3 possible resolving modes.

  • Closest package root to cwd (Current)
    Pros: If slides are managed in a monorepo, this way gives users the maximum flexibility.
    Cons: This forces users to use monorepo (with a monorepo tool like PNPM) if there are multi slides in one package.
  • Closest package root to slides.md
    Pros: No need to use a monorepo manager like PNPM, only nested package is required if there are multi slides in one package.
    Cons: Packages nested but not a monorepo is very confused.
  • Directory of slides.md (Before fix: package and path resolving #1308)
    Pros: Easy to understand
    Cons: In the future maybe we can build multiple slides together, and this is not very suitable for that.

We also need to find the user's package.json (https://sli.dev/addons/use#use-addon).

@antfu, I am not very sure.

@KermanX KermanX added the help wanted Extra attention is needed label Mar 9, 2024
@antfu
Copy link
Member

antfu commented Mar 9, 2024

I would prefer to keep the original directory of slides.md approach. Later in the slides.md we could have a

---
rootDir: ..
---

to override that if needed

@antfu antfu closed this as completed Mar 9, 2024
@antfu antfu reopened this Mar 9, 2024
Ynn pushed a commit to Ynn/slides.tools that referenced this issue Mar 9, 2024
2. Fix rooting issues : slidevjs/slidev#1401
3. Use node user
@KermanX KermanX self-assigned this Mar 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants