refactor: rewrite breadcrumbs to use Lit and add tests

[digitalsadhu]

I'm attempting to create a reference implementation for how best to write Lit Element based custom elements going forward. This includes a test setup and some basic tests.

Component Design

No Shadow DOM

As discussed previously, I've disabled Shadow DOM. We theorised in earlier discussions that there wasn't much to be gained at the component level by using the shadow dom (better at the podlet level) and not using it makes working with the Fabric CSS file easier. This also brings things more in line with the Vue and React implementations.

No slots

Not using the Shadow DOM does however mean that slots are no longer available. We can certainly work with this but it is somewhat of a shame since slots offer a powerful way to template.

Fabric CSS loaded globally

Since there's no component level isolation, there's no need to load the Fabric CSS file for every component. We can instead insist that Fabric Element users must include the Fabric CSS file in the page as we do with both Vue and React.

Global CSS

We discussed, during planning, the possibility of building inline styles per component. It's hard to see the benefit of this approach when we already build a single global package for all other use cases. We could inline per component which would likely result in very small inline css per component but when most pages will be including the global CSS file anyway, even a single kb extra is a waste. This is an area we could play around with more, however, in my mind this only really makes sense if we intend to keep Shadow DOM at the component level which so far we are leaning away from.

Idiomatic Lit

I've attempted to write in a Lit Idiomatic way. Additionally, I've created a base class as we discussed which actually does very little. It simply switches off Shadow DOM but could be used in future as needed. I'm a bit on the fence as to whether there's really much point in this base class to keen to hear feedback on this.

Testing

Web Test Runner and @open-wc/testing seem to be the recommended approach to testing Lit Element components. This works well, and was easy to setup.

I tried to use tap instead of the in built test framework stuff that comes included but this proved challenging.
It should be possible to implement but requires work that may or may not be worth the effort. See: https://modern-web.dev/docs/test-runner/test-frameworks/write-your-own/ This could be fun to try to get working for a hack day's project.

Web Test Runner wraps tests inside a browser context and can be configured to use puppeteer or playwright etc. See: https://modern-web.dev/docs/test-runner/browser-launchers/overview/

My final testing recommentation then is basically:

• web test runner to run the tests in a browser context and provides a mocha/chai test and assertion setup.
• @open-wc/testing to provide fixturing tools to Lit element components (allows you to easily fixture components onto the page)

[digitalsadhu]

intercept the child elements when the component is first added to the page and interleave them with slash character separators. Store the result as the internal property _children for use in the render method.

[pearofducks]

This interleaving is reactive in Vue (and I think React), but not here. Are we OK with that? Should that be disclosed?

[digitalsadhu]

Unsure. It is perhaps indicative of one of the differences between react/vue and custom elements. I toyed around with using dedicated child elements instead of this which is maybe one solution.

[pearofducks]

I'm fine with us making a call that people won't treat the children as reactive and fixing it later.

I think this is a lot easier to get into trouble with in Vue/React than it is manually monkeying with DOM elements. But I defer to your judgement here!

[benja]

When you say not reactive do you mean in the context of using this custom element in React or Vue, or as in if the user updates the props, it wouldn't update the DOM element? @pearofducks

[pearofducks]

I mean this implementation in the custom element isn't watching children - so if the user adds/removes breadcrumbs it won't re-interleave them.

[digitalsadhu]

The render method ignores the original children and instead renders out the interleaved ones stored in this._children

[digitalsadhu]

very simple base class that disables Shadow DOM. Is there much point in using this?

[digitalsadhu]

Tests look how you would expect but are run (by web test runner) in a browser context so that document is available and so on.

[trygve-lie]

Web Test Runner and @open-wc/testing seem to be the recommended approach to testing

What does these bring to the table over running ex plain PlayWright and ex tap?
That these are mocha/chai based rubs me the wrong way in so many ways.

[pearofducks]

Without slots, how do we approach a component like Modal and its footer slot/area? Is there an approach similar to what React does there?

[trygve-lie]

Whats the purpose of the [].concat here?

[digitalsadhu]

map creates pairs of elements and separators [el, separator] [el, separator] [el, separator], concat joins these array pairs together so you end up with 1 array with [el, separator, el, separator, el, separator]. Then the slice trims off the last separator so you end up with [el, separator, el, separator, el]

[pearofducks]

If we don't have to support Edge 16-18 or iOS11 we could instead array.flatMap(el => [el, separator]).slice(1)

[digitalsadhu]

Web Test Runner and @open-wc/testing seem to be the recommended approach to testing

What does these bring to the table over running ex plain PlayWright and ex tap? That these are mocha/chai based rubs me the wrong way in so many ways.

I hear you, I don't love it either but what you describe is not easy to setup. Playwright by itself as far as I can see is designed to test against some running page. It very much is not designed to support fixturing individual components yet which means we would need to do this ourselves. (See: microsoft/playwright#7148 for example)

To build something on top of tap and playwright it seems like we would need to write something to serve a page and support setting up and tearing down the page for each test and then something to add fixture the components to the page. In each test you would then use the playwright apis to write tests agains the fixtured page. At that point it's probably easier to just use jsdom in tap.

Lit recommends web test runner: https://lit.dev/docs/tools/testing/
Doesn't seem easy to get working with Tap out of the box though.

EDIT: I guess if we were to engineer something from the ground up, nicest would probably be a lib that creates a fastify server that is started on a random port for each test and has apis for stop, start, fixturing components etc. Then as part of your playwright test you would write something like this:

test('test a component', () => {
    const server = await fixtureServer();
    const component = html`<my-component></my-component>`;
    server.fixture(component);

   await page.goto(server.url);
   const title = page.locator('.navbar__inner .navbar__title');
   await expect(title).toHaveText('whatever');
});

[digitalsadhu]

Without slots, how do we approach a component like Modal and its footer slot/area? Is there an approach similar to what React does there?

Yea, its a bit of a loss for sure. Probably a couple ways you could approach it, some more hacky than others. You could provide component building blocks devs should use so we can grab them out of the children and place them appropriately for example.

Eg.

<f-modal>
  <f-modal-header>stuff</f-modal-header>
  <f-modal-content>more stuff</f-modal-content>
  <f-modal-footer>even more stuff</f-modal-footer>
</f-modal>

Slots would be nicer.

[trygve-lie]

No Shadow DOM
As discussed previously, I've disabled Shadow DOM.

Just so I understand the term here. Is what we call "disabling Shadow DOM" the same as defining Shadow DOM to be open? Or are we "disabling" it in another way?

[digitalsadhu]

No Shadow DOM
As discussed previously, I've disabled Shadow DOM.

Just so I understand the term here. Is what we call "disabling Shadow DOM" the same as defining Shadow DOM to be open? Or are we "disabling" it in another way?

No, we are talking about disabled vs enabled, not open vs closed Shadow DOM.

Ie. We are turning Shadow DOM off for the components rather than enabling Shadow DOM but deciding whether to use open or closed mode.

In Lit you can write your components without Shadow DOM by adding:

createRenderRoot() {
  return this;
}

By default, Lit creates the Shadow DOM in open mode and, as I understand it, there's very little value in using the closed Shadow DOM. Ref: https://blog.revillweb.com/open-vs-closed-shadow-dom-9f3d7427d1af

Lit gives you access to a property called .renderRoot which will be the shadowRoot if the component is using Shadow DOM or the component itself otherwise. This ensures that this.renderRoot.querySelector etc. always works regardless of whether the component is using Shadow DOM or not.

[digitalsadhu]

Web Test Runner and @open-wc/testing seem to be the recommended approach to testing

What does these bring to the table over running ex plain PlayWright and ex tap? That these are mocha/chai based rubs me the wrong way in so many ways.

I hear you, I don't love it either but what you describe is not easy to setup. Playwright by itself as far as I can see is designed to test against some running page. It very much is not designed to support fixturing individual components yet which means we would need to do this ourselves. (See: microsoft/playwright#7148 for example)

To build something on top of tap and playwright it seems like we would need to write something to serve a page and support setting up and tearing down the page for each test and then something to add fixture the components to the page. In each test you would then use the playwright apis to write tests agains the fixtured page. At that point it's probably easier to just use jsdom in tap.

Lit recommends web test runner: https://lit.dev/docs/tools/testing/ Doesn't seem easy to get working with Tap out of the box though.

EDIT: I guess if we were to engineer something from the ground up, nicest would probably be a lib that creates a fastify server that is started on a random port for each test and has apis for stop, start, fixturing components etc. Then as part of your playwright test you would write something like this:

test('test a component', () => {
    const server = await fixtureServer();
    const component = html`<my-component></my-component>`;
    server.fixture(component);

   await page.goto(server.url);
   const title = page.locator('.navbar__inner .navbar__title');
   await expect(title).toHaveText('whatever');
});

I managed to prototype up a fastify test server that mounts components like so:

import fs from 'fs';
import fastify from 'fastify';

export class Server {
    _component = '';

    constructor() {
        this.app = fastify();
        this.app.get('/scripts.js', function (request, reply) {
            const stream = fs.createReadStream(new URL('../../dist/index.js', import.meta.url), 'utf8');
            reply.type('application/javascript');
            reply.send(stream);
        });

        this.app.get('/', (req, reply) => {
            reply.type('text/html');
            reply.send(`
                <html>
                    <head>
                        <link href="https://assets.finn.no/pkg/@fabric-ds/css/v1/fabric.min.css" type="text/css" rel="stylesheet" />
                        <script type="module">
                            import "./scripts.js";
                            import { html, render } from 'https://cdn.skypack.dev/lit';
                    
                            render(
                                html\`${this._component}\`,
                                document.querySelector('body')
                            )
                        </script>
                    </head>
                    <body></body>
                </html>
            `);
        });
    }

    fixture(markup) {
        this._component = markup;
    }

    async start() {
        return this.app.listen('4021', '0.0.0.0');
    }

    async stop() {
        return this.app.close();
    }
}

You then use it like this:

import { test } from 'tap';
import { Server } from './server.js';

const server = new Server();
await server.start();

test('Breadcrumb component renders on the page', async () => {
  // GIVEN: A component with 1 breadcrumb
  const component = `<f-breadcrumbs
    ><a href="#/url/1">Eiendom</a></f-breadcrumbs
  >`;

  // WHEN: the component is added to the page
  server.fixture(component);

  // THEN: the component is visible in the DOM
  // use playwright to assert tests against the page located at http://localhost:4021/
});

Thoughts @trygve-lie ?

[trygve-lie]

No, we are talking about disabled vs enabled, not open vs closed Shadow DOM.
Ie. We are turning Shadow DOM off for the components rather than enabling Shadow DOM but deciding whether to use open or closed mode.

Ok. Which brings me to:

We theorised in earlier discussions that there wasn't much to be gained at the component level by using the shadow dom (better at the podlet level) and not using it makes working with the Fabric CSS file easier.

I think we need to address this first. Our main challenge here is actually aligning / sharing CSS between the components and aligning / sharing CSS between the document and components. So there is actually two challenges here.

The first challenge is to share CSS between all web components we build. This is doable and is done by having a component exporting all CSS:

import { css } from 'lit';

export const buttonStyles = css`
  .blue-button {
    color: white;
    background-color: blue;
  }
  .blue-button:disabled {
    background-color: grey;
  }`;

And then all web components must import this component:

import { buttonStyles } from './button-styles.js';

class MyElement extends LitElement {
  static styles = [
    buttonStyles,
    css`
      :host { display: block;
        border: 1px solid black;
      }`
  ];
}

What I imagine is that we publish a Tailwind Web Component which exports all the styles and all components then import this. To do so we probably need a custom build step / tool to build this component but I imagine it being pretty doable.

The above will make sure that all Web Components will have one CSS loaded once which all use. Each component will not have to load or bundle its own set of CSS.

This leave us with one problem though; the Web Components will have its own styles in JS while the rest of the document will pull in the styles as CSS. This mean that a page with a Web Component on it will load the same CSS twice. If we able to make it so that Web Components and the document can use the same CSS (loaded once) in a page, we can use Shadow DOM in all Web Components and then also slots.

There is no defined way of doing this, but I have a couple of ideas on how we can approach it but I am not sure discussing that belongs in this PR.

[digitalsadhu]

I've pushed a change to the test setup that shows using Tap and Playwright. It seems to work pretty nicely. There is a server.js file that sets up a fastify server to fixture components and serve them on a page and configure playwright to test against this page. Interested to hear your thoughts on this approach @trygve-lie

[digitalsadhu]

No, we are talking about disabled vs enabled, not open vs closed Shadow DOM.
Ie. We are turning Shadow DOM off for the components rather than enabling Shadow DOM but deciding whether to use open or closed mode.

Ok. Which brings me to:

We theorised in earlier discussions that there wasn't much to be gained at the component level by using the shadow dom (better at the podlet level) and not using it makes working with the Fabric CSS file easier.

I think we need to address this first. Our main challenge here is actually aligning / sharing CSS between the components and aligning / sharing CSS between the document and components. So there is actually two challenges here.

The first challenge is to share CSS between all web components we build. This is doable and is done by having a component exporting all CSS:

import { css } from 'lit';

export const buttonStyles = css`
  .blue-button {
    color: white;
    background-color: blue;
  }
  .blue-button:disabled {
    background-color: grey;
  }`;

And then all web components must import this component:

import { buttonStyles } from './button-styles.js';

class MyElement extends LitElement {
  static styles = [
    buttonStyles,
    css`
      :host { display: block;
        border: 1px solid black;
      }`
  ];
}

What I imagine is that we publish a Tailwind Web Component which exports all the styles and all components then import this. To do so we probably need a custom build step / tool to build this component but I imagine it being pretty doable.

The above will make sure that all Web Components will have one CSS loaded once which all use. Each component will not have to load or bundle its own set of CSS.

This leave us with one problem though; the Web Components will have its own styles in JS while the rest of the document will pull in the styles as CSS. This mean that a page with a Web Component on it will load the same CSS twice. If we able to make it so that Web Components and the document can use the same CSS (loaded once) in a page, we can use Shadow DOM in all Web Components and then also slots.

There is no defined way of doing this, but I have a couple of ideas on how we can approach it but I am not sure discussing that belongs in this PR.

Cool, makes sense. Interested to hear what possible solutions to avoid duplication you have come up with. Have to admit, in my head this seems like a tricky problem.

[digitalsadhu]

There's lots that could be done to generalise this file but for now its a quick and dirty prototype which proves the setup.

[digitalsadhu]

I've updated this PR to use the constructed stylesheet approach we have discussed. For this change to work, this PR needs to be merged and published: fabric-ds/css#68

[digitalsadhu]

The latest on the custom element prototyping...

There are 3 areas to talk about:

• Sharing Fabric styles efficiently and without FOUC and accounting for SSR
• Testing
• Sharing logic

I have now done quite a lot of experimentation with Lit and SSR. I have made several working prototypes and experimented with different approaches to including (or building) Fabric and I have come to the following conclusions:

1. CSSStyleSheet and import asserts are not going to work in an acceptable manor with SSR anytime soon if at all. CSSStyleSheet would require some form of shimming the browser objects in node land AND would need Lit to take a different approach to how it does SSR (which it may some day, who knows but they ain't today). The Lit SSR approach ALWAYS inlines all styles into a <style> tag in the components markup which no matter how you build Fabric (including purging in components) means a lot of extra kb and the loss of the advantages of inline asserts or CSSStyleSheet objects which are aimed at sharing references between components.
2. Having done some testing and more reading, I've discovered and curious and useful fact. If you use @import url inside a style tag inside your component markup INSTEAD of using link tags, the component will not render before the CSS is in place so you can completely eliminate FOUC from the component. Technically, this could have performance issues but in practice, can be mitigated by preloading the Fabric CSS file in the page which also the main solution to avoiding FOUC when using link tags which don't block. Using imports takes us to a "works by default" and then "optimise perf by preloading" approach instead of "FOUC by default" and then "get rid of the FOUC by preloading"
3. If you do not support SSR in Lit components, using a CSSStyleSheet object approach to share references would be very compelling. Currently import with CSS asserts are not well supported across browsers though there are some potential shimming options. eg. https://github.com/guybedford/es-module-shims#es-module-shims
4. If we do support SSR in Lit components, I think we will need to stick with including the global Fabric CSS file (preferably via <style>@import url(...)</style>. This approach has shown itself to be pretty solid in my latest lot of testing.
5. On testing, we have now successfully managed a Playwright/Tap test setup that you can see in this PR so that's ready to go
6. Dave and Benjamin have been working on sharing logic across component libraries which means we will be hopefully be able to end up with consistent implementations across the board. (Slider implementation here: https://github.com/fabric-ds/vue/blob/158137ac8c0365896d8286330d8a8c6860968158/components/slider/handlers.js)

You can see some POCs for different approaches to Lit SSR here: https://github.schibsted.io/richard-walker/ssr_lit_podlet

Specifically:

• inline link tag: https://github.schibsted.io/richard-walker/ssr_lit_podlet/blob/473b4470d41c14954df179bd0d95ebac819d89b4/inline-link-tag/app.js#L19
• Fabric Tailwind purge approach: https://github.schibsted.io/richard-walker/ssr_lit_podlet/blob/main/inline-style-rules-purge/app.js
• CSSStyleSheet/lit-html JS object exported from Fabric CSS approach: https://github.schibsted.io/richard-walker/ssr_lit_podlet/blob/main/inline-style-rules/app.js

Final Summary
We need to move forward and I believe we have enough information to get started. We can always come back and relook at how Fabric is included and I doubt it will ever be that hard to refactor if necessary.

For now, I believe we should:

• Merge this PR
• Create a base class that imports the global Fabric file using <style>@import url('https://assets.finn.no/pkg/@fabric-ds/v1/fabric.min.css')</style>
• Make sure its clear that people should preload Fabric on their pages (they should be anyway).
• Write tests using Tap and Playwright
• Share logic with the other components with the other libraries as best we can.

[github-actions]

🎉 This PR is included in version 0.1.3-next.1 🎉

The release is available on:

• npm package (@next dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀