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

Create JS library paths relative to HTML #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

agarzola
Copy link

@agarzola agarzola commented Mar 7, 2019

TL;DR

This PR modifies the attach_library function to produce JS library paths that are relative to the generated component HTML documents, rather than to the root of the site.

(Long-winded) Background

We’re using Emulsify and have CI automatically building a static version of Pattern Lab so developers and stakeholders can review the patterns on the live environment. This saves us having to keep a separate Node process running to serve from the theme directory. We access the pattern library via a URL that looks like this:

https://project-domain.com/themes/custom/THEME_NAME/pattern-lab/public

To accomplish this, our deployment script simply navigates to the theme directory, installs dependencies, and invokes a script that builds static files, like so:

cd web/themes/custom/THEME_NAME && npm ci --usafe-perm && npm run build

Where the build script in our theme’s package.json is as follows:

"scripts": {
  "build": "gulp build",
  // other scripts
}

Note that gulp build simply invokes this task in emulsify-gulp. The result is a navigable set of static Pattern Lab files that point to ../../../../dist/style.css for CSS and script tags that point to /dist/path_to_component/component_name.js, i.e. while CSS paths are relative to the HTML document, JS paths are relative to the root of the site.

To illustrate the problem with this in our scenario, here’s that sample URL again with some notes:

                                                   ▼ dist/ lives at this level
https://project-domain.com/themes/custom/THEME_NAME/pattern-lab/public
                          ▲ <script> tags are written as if it lived here

Note that using paths relative to the root of the site works correctly when serving Pattern Lab from the theme directory (i.e. when using npm start) because dist/ happens to be at the root level in that context. However, this is brittle, because it requires Pattern Lab to be served from the theme directory in order for JS paths to work.

This PR addresses this problem by producing JS library paths that, much like the style.css path, are relative to the component’s HTML file, and not to the root of the site, which enables us to access Pattern Lab in the way described above while continuing to support the server run by Emulsify’s npm start.

This is a more resilient solution since it doesn’t rely on Pattern Lab
being served from the theme directory.
@waako
Copy link

waako commented Mar 7, 2019

Exactly what we have done with our themes, set relative path ../../../../ as mentionned in comment on #2 as that didn't work for me.

I'm glad same works for others in different scenario, as wasn't certain if it was just our setup or depth of components that meant the 4 level relative prefix worked.

@agarzola
Copy link
Author

agarzola commented Mar 7, 2019

Thanks for linking to that PR, @waako. I hadn’t noticed #2, but I do think this solution is simpler, since —as far as I can tell— dist/ is always four levels above any component’s generated HTML file:

      4          3      2                       1
      ▼          ▼      ▼                       ▼
theme/pattern-lab/public/component-type-and-name/generated-component-markup.html
theme/dist/

Although I could be missing something. My experience with this is limited to its use within Emulsify.

@waako
Copy link

waako commented Mar 13, 2019

What I was trying to say is that I completely agree with you and this PR, it seems to be the most effective and simple way and works for us too.

@agarzola
Copy link
Author

@waako I did get that you agree with this PR! 😄

Since you pointed out #2, I figured it would be good to expand a bit on the reasoning behind the hard-coded ../../../../ in case other folks were comparing the two PRs. Apologies, I did not mean for it to sound defensive!

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.

2 participants