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

Compile assets before uploading #1

Closed
m1guelpf opened this issue Oct 22, 2019 · 15 comments · Fixed by #11
Closed

Compile assets before uploading #1

m1guelpf opened this issue Oct 22, 2019 · 15 comments · Fixed by #11

Comments

@m1guelpf
Copy link

My theme needs to build assets before uploading. It would be nice to add an option to run gulp/webpack whatever or, as an alternative, accept an existing zip file instead of creating one.

@d3473r
Copy link

d3473r commented Nov 1, 2019

+1 on this. Tracking the compiled js files with git is not best practice.
https://github.com/eddiesigner/liebling this theme needs to build the js files before uploading

@mattorb
Copy link

mattorb commented Nov 16, 2019

I just started using this action. I forked Casper to start my theme.

I added a custom .js file to my theme assets and checked it in.

I did not realize I would be expected to check in the minified version of the asset. Seems like the deploy could maybe build that if needed during the deploy process -- rather than requiring it to be checked in to vcs.

@pascalandy
Copy link

pascalandy commented Jan 7, 2020

+1. I'd like to recreate this process as well:

  • I do gulp build to build my theme.
  • It creates a /dist directory.
  • I zip /dist manually
  • upload the zip file on Ghost (3.1.1) manually and it’s all good.

@jloh
Copy link
Member

jloh commented Jan 7, 2020

Hi all,

This is already possible as-is, you just need to add more steps to your workflow config. Here is an example one that I use to run gulp:

name: Deploy Theme
on: [push]

jobs:
  deploy:
    runs-on: ubuntu-18.04
    steps:
      - uses: actions/checkout@master
      - name: Cache node modules
        uses: actions/cache@v1
        with:
          path: node_modules
          key: ${{ runner.OS }}-build-${{ hashFiles('**/package-lock.json') }}
          restore-keys: |
            ${{ runner.OS }}-build-${{ env.cache-name }}-
            ${{ runner.OS }}-build-
            ${{ runner.OS }}-
      - name: Use Node.js ${{ matrix.node-version }}
        uses: actions/setup-node@v1
        with:
          node-version: 8.11.1
      - run: npm install
      - name: Build theme
        env:
          NODE_ENV: production
        run: npx gulp
      - name: 'Deploy Ghost Theme'
        uses: TryGhost/action-deploy-theme@v1.2.0
        with:
          api-url: ${{ secrets.GHOST_ADMIN_API_URL }}
          api-key: ${{ secrets.GHOST_ADMIN_API_KEY }}
          theme-name: jloh-personal

@kevinansfield
Copy link
Contributor

kevinansfield commented Jan 8, 2020

The approach @jloh has taken here is the correct one IMO. Ghost's theming system leaves the build environment (if there is one) up to the theme developer so we're not really in a position for the deployment action to dictate how a theme needs to be built.

It may change in the future if we standardise on themes using a Node.js-based build and including a specific build script in their package.json but for now anyone can use this action as-is as part of their github workflow.

@pascalandy
Copy link

pascalandy commented Jan 8, 2020

I just tried yml's file from @jloh.

The problem is that I still encounter the "Error: Request body larger than maxBodyLength limit". I detailed it here. Yes my theme is big ==> 50MO fully compressed and minified. There is a lot of custom pages, templates, tags, images with 3 resolutions, etc. You know, like most corpo sites.

To avoid any glitch between local dev and the CI, I feel the best solution is the build, make a zip file for the team and then, let the CI upload on our website.

At the moment, the upload part is the only thing missing. See my wip CI.yml

Thanks!

@kevinansfield
Copy link
Contributor

@pascalandy that is an issue with the @tryghost/admin-api package and it's underlying axios dependency rather than being directly related to what the action is doing. I've opened an issue on the relevant repo here.

I feel the best solution is the build, make a zip file for the team and then, let the CI upload on our website

That's fair enough but I'd suggest adding the ability for this action to accept a zip is a different feature request than this issue 🙂 In your case it may be simpler to do it manually in your custom workflow, it's only 1 line after all 😉 https://github.com/TryGhost/action-deploy-theme/blob/master/index.js#L26

@d3473r
Copy link

d3473r commented Jan 8, 2020

@jloh and @kevinansfield thanks for the hints :)
I am using the zipping mechanism of the theme and then upload it with the admin-api

.github/workflows/deploy-theme.yml

name: Deploy Theme
on: [push]

jobs:
  deploy:
    runs-on: ubuntu-18.04
    steps:
      - uses: actions/checkout@master
      - uses: actions/setup-node@v1
        with:
          node-version: '10.x'
      - run: npm install
        working-directory: ./src
      - name: Build theme
        working-directory: ./src
        run: npm run production
      - name: 'Deploy Ghost Theme'
        run: node src/deploy/index.js
        env:
          url: ${{ secrets.GHOST_ADMIN_API_URL }}
          key: ${{ secrets.GHOST_ADMIN_API_KEY }}

src/deploy/index.js

const GhostAdminApi = require('@tryghost/admin-api');

(async function main() {
    try {
        const api = new GhostAdminApi({
            url: process.env.url,
            key: process.env.key,
            version: 'canary'
        });

        // Deploy it to the configured site
        await api.themes.upload({file: 'liebling.zip'});
        console.log('Theme successfully uploaded.');
    } catch (err) {
        console.error(err);
        process.exit(1);
    }
}());

@pascalandy
Copy link

@d3473r may I DM you? I'm asking as your DM are closed on twitter. Thanks!

@pascalandy
Copy link

pascalandy commented Jan 9, 2020

@d3473r I forked your theme to test the CI. I saw the CI did push the theme on my site. As expected, using my theme (50MB), I get the same error as explained here.

Question: do you see an advantage of using your set up over the CI provided by the Ghost's team (https://github.com/TryGhost/action-deploy-theme)?

@d3473r
Copy link

d3473r commented Jan 9, 2020

@d3473r may I DM you? I'm asking as your DM are closed on twitter. Thanks!

You can DM me now on Twitter.
Not really an advantage, just more suited for the theme as it already contains a CI mechanism.
For users who simply are tracking their final theme in git this action is perfect.

@pascalandy
Copy link

Thanks for letting me know!

Not really an advantage, just more suited for the theme as it already contains a CI mechanism.

@vikaspotluri123
Copy link
Member

@kevinansfield coming back to the original issue, I've created a version in my fork that accepts a file to be uploaded, which would solve

as an alternative, accept an existing zip file instead of creating one.

Should I clean up the changes and create a PR for it?

@kevinansfield
Copy link
Contributor

@vikaspotluri123 go for it! 😄

1 similar comment
@kevinansfield
Copy link
Contributor

@vikaspotluri123 go for it! 😄

vikaspotluri123 added a commit to vikaspotluri123/action-deploy-theme that referenced this issue Feb 27, 2020
closes TryGhost#1

- Add a new `file` option which uploads the specified file instead of generating one
daniellockyer pushed a commit that referenced this issue May 19, 2020
closes #1

- Add a new `file` option which uploads the specified file instead of generating one
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 a pull request may close this issue.

7 participants