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

Clarify how the docs / Playground deployment process works #616

Closed
TomasHubelbauer opened this issue May 29, 2023 · 8 comments · Fixed by #619
Closed

Clarify how the docs / Playground deployment process works #616

TomasHubelbauer opened this issue May 29, 2023 · 8 comments · Fixed by #619

Comments

@TomasHubelbauer
Copy link
Contributor

TomasHubelbauer commented May 29, 2023

I am trying to learn how to test my changes in the Playground when running locally. For this I am working through the build process to understand how all the pieces go together to make the Playground build and serve and use the built Liquid version.

I think the process of building the Playground is rather simple:

  • npm run build:docs calls build-docs.sh and it goes to docs and runs npm ci followed by npm run build
  • npm run build in docs just calls hexo generate and that's how we get public/playground.html and public/js/main.js
    • playground.html is just compiled playground.pug
    • index.js is copied over from themes/navy/source/js/main.js

The the docs site can be run locally by running npm start in docs.
At this point I know how the HTML of the site is built and I know the JS controlling the stuff like reacting to the changes in the editor and running the source code there through Liquid.

But as far as I can see, main.js just touches the liquidjs global.
And the only way I can see that referenced is in the HTML like so:

<script src="https://cdn.jsdelivr.net/npm/liquidjs/dist/liquid.browser.min.js"></script>

This is from docs/themes/navy/layout/partial/after_footer.swig.

Does this mean the local version of the site still uses LiquidJS from NPM and not the local build in dist?
https://www.npmjs.com/package/liquidjs

I am trying to figure out because even though I contributed #611 and the Docs GitHub Actions workflow ran successfully, the live site of the Playground doesn't seem to work any differently, push there still doesn't work. I see the last release on NPM is from a month ago, so it wouldn't contain my changes I made and explain the situation.

If I understood everything correctly, it means you might need to cut a new NPM release in order for #611 and #612 to work on the live Playground site, right? Could you please do that?

And do you have any tips or tricks for using the local build of LiquidJS in the Playground? One of the things I am wondering about is whether once the new NPM version is released and the live site is made to use it, whether it will correctly highlight the push filter name like it does for other filters. If there is work that needs to be done to support that, I will be happy to do it, but I need to make sure so far I've understood everything correctly.

@harttle harttle added the docs label May 30, 2023
@harttle
Copy link
Owner

harttle commented May 30, 2023

You're right, playground now relies on the latest npm package which is extracted/served by jsdelivr.

To test it locally, you'll need to copy the JS from dist to the docs directory and change that <script> src accordingly. But I do think we need allow easier local test for Playground. We can include a copy of dist JS in the docs build, so it should work both locally and on pipeline.

I'm working on the next npm package release in which I want #618 be included and test coverate to 100% again. Please expect this week.

whether it will correctly highlight the push?

This depends on ace. You'll need to contribute to their highlight lib to add new filters. Or, let's see whether we can define new keywords using existing API.

@TomasHubelbauer
Copy link
Contributor Author

Thanks! I've opened #619 to contribute instructions on how to build the Playground site and point it to the local build of LiquidJS.

Regarding Ace, I didn't know it, so I took a skim through their documentation on syntax highlighting:
https://ace.c9.io/#nav=higlighter

They call syntax highlighters "modes" and show how to add and extend them.
I was able to find the place in the LiquidJS codebase where Ace is initialized and the mode is set:

docs/themes/navy/source/js/main.js

function createEditor(id, lang) {
  const editor = ace.edit(id);
  editor.setTheme('ace/theme/tomorrow_night');
  editor.getSession().setMode('ace/mode/' + lang);
  editor.getSession().setOptions({
    tabSize: 2,
    useSoftTabs: true
  });
  editor.renderer.setScrollMargin(15);
  return editor;
}

This is then called elsewhere in the same file to initialize the three Playground editors:

const editor = createEditor('editorEl', 'liquid');
const dataEditor = createEditor('dataEl', 'json');
const preview = createEditor('previewEl', 'html');

Here we can see we set the liquid, json and html modes respectively.
The Liquid mode is found here in the Ace repository:
https://github.com/ajaxorg/ace/blob/master/src/mode/liquid.js

The syntax highlighting rules specifically are found here:
https://github.com/ajaxorg/ace/blob/master/src/mode/liquid_highlight_rules.js

I tried figuring out how to adjust these files to make the filters I added highlight, but it is confusing to me how the rules even work. I decided to ask a discussion question in the Ace project:
ajaxorg/ace#5189

@harttle if you happen to know the answer please let me know, in the meanwhile I will monitor the Ace project discussion question I created and will try to figure out how to add support for the new filter names in to the Liquid code highlighter.

@TomasHubelbauer
Copy link
Contributor Author

Actually, I just tried pasting the code with the new filter name to the Ace kitchen sink demo page here: https://ace.c9.io/build/kitchen-sink.html. When I select Liquid as the mode there, both old and new filter names get highlighted the same way there.

I figured maybe the theme had some effect on things so I removed this line: editor.setTheme('ace/theme/tomorrow_night');.

This switched the Ace instance in the Playground to the default theme, same as the one on the Ace Kitchen Sink site. Even in this case, the push (new) filter name was not highlighted but split (existing) filter name was.

Can you think of why the discrepancy?

Playground without theme (push is not highlighted, split is):
image

Ace Kitchen Sink (both filter names are highlighted):
image

The difference is subtle, but you can see on the Ace site both names are highlighted but on the LiquidJS Playground site, only one is.

Here is the test code I am using in case you want to try to reproduce the issue:

{% assign people = "alice, bob, carol" | split: ", " -%}
{% assign people = people | push: "tom" %}

<ul>
{%- for person in people %}
  <li>
    <a href="{{person | prepend: "http://example.com/"}}">
      {{ person | capitalize }}
    </a>
  </li>
{%- endfor%}
</ul>

@github-actions
Copy link

github-actions bot commented Jun 3, 2023

🎉 This issue has been resolved in version 10.8.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@harttle
Copy link
Owner

harttle commented Jun 3, 2023

@TomasHubelbauer I believe the latest ace version addresses that issue. Just updated to latest and it works on my local server.

@TomasHubelbauer
Copy link
Contributor Author

That's awesome, thank you! I can confirm it works perfectly now for me as well:
image

I noticed that in the live Playground it doesn't work yet and it still reports the version as 10.7.1. Do you see the same?
The URL in after_footer.swig is this:

https://cdn.jsdelivr.net/npm/liquidjs/dist/liquid.browser.min.js

I checked and this returns the 10.7.1 source.

If we use this URL it returns 10.8.0:

https://cdn.jsdelivr.net/npm/liquidjs@latest/dist/liquid.browser.min.js

This is the same as (at the time of writing):

https://cdn.jsdelivr.net/npm/[email protected]/dist/liquid.browser.min.js

Do you know why without @latest it still defaults to 10.7.1?
I wonder if a PR to change it to the URL with @latest would be helpful?

I am wondering though it maybe it just takes JSDelivr a while to switch the first URL to the latest version, not sure…

@harttle
Copy link
Owner

harttle commented Jun 4, 2023

I am wondering though it maybe it just takes JSDelivr a while to switch the first URL to the latest version

I guess so.

I wonder if a PR to change it to the URL with @latest would be helpful?

I agree, it’d be better. Maybe there’ll a subtle performance regression but I think it’s perfectly OK.

@harttle
Copy link
Owner

harttle commented Jun 4, 2023

Updated it to latest in one of my commits. It changes to the latest but I'm not sure whether it still be cached as stated in its response header. We'll see next time release a version.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants