Theme colors are broken for direct field access

[bebraw]

Here's a simple repro against your online environment:

// Oceanwind demo by @lukejacksonn
// ----------------

import { render, h } from 'https://unpkg.com/preact?module';
import htm from 'https://unpkg.com/htm?module';
import { themed } from 'https://unpkg.com/oceanwind';

const html = htm.bind(h);

const ow = themed({
  colors: {
    primary: 'orange',
    red: {
      500: 'hotpink',
    },
  },
});

render(
  html`
    <div className=${ow`
      h-full
      bg-purple-500
      flex
      items-center
      justify-center
    `}>
      <h1 className=${ow`
        bg-red-500
        text-primary
      `}>Hello World</h1>
    </div>
  `,
  document.body
);

[bebraw]

Here's a potential fix.

Test:

import assert from 'assert';
import { h } from 'preact';
import render from 'preact-render-to-string';
import htm from 'htm';
import {
  setup,
  themed,
  filterOutUnusedRules,
  getStyleTag,
  VirtualInjector,
} from '../index.mjs';

function testTheme() {
  // https://www.npmjs.com/package/otion#server-side-rendering
  const injector = VirtualInjector();
  setup({ injector });

  const html = htm.bind(h);
  const ow = themed({
    colors: {
      primary: 'orange',
      red: {
        500: 'hotpink',
      },
    },
  });
  const style = {
    testOne: ow`bg-primary text-primary`,
    testTwo: ow`bg-red text-red`,
  };

  const app = html`<div className=${style.testOne} />
    <div className=${style.testTwo} />`;
  const appHtml = render(app);
  const styleTag = getStyleTag(filterOutUnusedRules(injector, appHtml));

  console.log('Generated a style tag', styleTag);

  assert(!styleTag.includes('primary'));
}

testTheme();

Then change to out['color'] = theme.colors[i[1]] || i[1] at translate (two spots at least).

It's possible there are more places to test and check.

I can send a PR for this after we land the SSR one as that contains the basic test infrastructure. I guess we could try to generate some tests for these as it's the ideal use case for property based testing.

[lukejacksonn]

I didn't actually know that accessing colors directly was valid - probably because I've not used colors.primary before. A fix should be trivial enough to implement. We should look into it after we have merged the SSR PR though like you say!

[bebraw]

Yup, it’s a trivial fix (fixed for myself already :) ).

On 14. Sep 2020, at 0.41, Luke Jackson notifications@github.com wrote:

 I didn't actually know that accessing colors directly was valid - probably because I've not used colors.primary before. A fix should be trivial enough to implement. We should look into it after we have merged the SSR PR though like you say!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[lukejacksonn]

Fixed by #17