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

Implement GitHub Pages plugin #409

Merged
merged 5 commits into from
Nov 16, 2023
Merged

Implement GitHub Pages plugin #409

merged 5 commits into from
Nov 16, 2023

Conversation

alexdavid
Copy link
Contributor

No description provided.

@alexdavid alexdavid linked an issue Nov 15, 2023 that may be closed by this pull request
@alexdavid alexdavid force-pushed the sh/gh-pages-plugin branch 5 times, most recently from 364a0d5 to adf7bb0 Compare November 16, 2023 16:55
@alexdavid
Copy link
Contributor Author

This is ready for review. A few changes that I made that I don't feel super strongly about, but would like your thoughts on @soenkehahn:

  • I moved the plugin to the root garn export, it felt strange that we nested it under javascript since you might have a static html site that you want to deploy to GitHub pages.
  • I removed the Config type, instead opting to pass the project in directly to the first param. The thinking here is that while we may have additional options in the future, the only required thing I think we will ever need is the package, and I think this reads a bit nicer (and made the JSDoc read nicer as well since we don't have a great way of linking to the config type in deno doc):
.add(deployGhPages(self => self.myPkg))
// and in the future:
.add(deployGhPages(self => self.myPkg, { pushTo: 'origin' }))

rather than

.add(deployGhPages(self => ({ package: self.myPkg })))
// in the future:
.add(deployGhPages(self => ({ package: self.myPkg, pushTo: 'origin' })))

especially since I can't imagine any use case for any options needing something off of the Project, but let me know if you prefer the other way.

@alexdavid alexdavid marked this pull request as ready for review November 16, 2023 17:05
Copy link
Contributor

@soenkehahn soenkehahn left a comment

Choose a reason for hiding this comment

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

I tried this out on a project here: https://github.com/soenkehahn/garn-example-vite-vanilla (This is using a branch of garn with both the gh-pages plugin and the vite-builder plugin.)

When doing garn run viteVanilla.deployToGhPages in there, it creates a commit on gh-pages, but I get this output:

[...]
Created commit to "gh-pages" branch, but it has not been pushed yet
Run git push <remote> gh-pages:gh-pages to deploy
rm: cannot remove '/tmp/tmp.Mn58cn7bio/dst/assets/index-QFzqfbCu.js': Permission denied
rm: cannot remove '/tmp/tmp.Mn58cn7bio/dst/assets/index-s-MxoWxy.css': Permission denied

I'm assuming the error messages at the end are caused by copying something from the nix store to a temporary directory, which then will be read-only. And then trying to delete the temporary directory afterwards.

Also, when pushing to the gh-pages branch, github serves the files, however the app doesn't work because of mime-type issues. :( Maybe this is something that has to be fixed in the vite-builder plugin?

.github/workflows/ci.yaml Outdated Show resolved Hide resolved
ts/deployToGhPages.test.ts Outdated Show resolved Hide resolved
ts/deployToGhPages.test.ts Outdated Show resolved Hide resolved
const ansiRedBold = "\\e[31;1m";

/**
* A garn plugin that allows easy deployment of a package to GitHub pages.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe link to github pages documentation here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ts/deployToGhPages.ts Outdated Show resolved Hide resolved
@soenkehahn
Copy link
Contributor

Also, when pushing to the gh-pages branch, github serves the files, however the app doesn't work because of mime-type issues. :( Maybe this is something that has to be fixed in the vite-builder plugin?

Never mind. To deploy vite apps to github pages, you have to configure the base config variable, see https://vitejs.dev/guide/static-deploy#github-pages.

Copy link
Contributor

@soenkehahn soenkehahn left a comment

Choose a reason for hiding this comment

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

Looks good, apart from my comments.

Co-authored-by: Sönke Hahn <[email protected]>
@alexdavid
Copy link
Contributor Author

I'm assuming the error messages at the end are caused by copying something from the nix store to a temporary directory, which then will be read-only. And then trying to delete the temporary directory afterwards.

Yeah, our test package didn't output any nested directories. I updated the tests in c32d5a4 to create a nested dir to catch this in the future.

Never mind. To deploy vite apps to github pages, you have to configure the base config variable, see https://vitejs.dev/guide/static-deploy#github-pages.

One thing to note is that this only applies to project pages, not user/organization pages.

@alexdavid alexdavid enabled auto-merge (squash) November 16, 2023 21:59
@alexdavid alexdavid merged commit 9710bb2 into main Nov 16, 2023
28 checks passed
@alexdavid alexdavid deleted the sh/gh-pages-plugin branch November 16, 2023 22:14
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.

Allow to easily deploy to gh_pages
2 participants