ember-component-css icon indicating copy to clipboard operation
ember-component-css copied to clipboard

Doesn't work in Octane (3.15) glimmer component

Open BryanCrotaz opened this issue 5 years ago • 18 comments

in the template, class={{styleNamespace}}, {{@styleNamespace}} and {{this.styleNamespace}} all evaluate to no class attribute.

BryanCrotaz avatar Jan 03 '20 10:01 BryanCrotaz

To get it to work you need a backing component class.

import Component from "@glimmer/component";
import podNames from "ember-component-css/pod-names";

export default class MyComponentName extends Component {
  get styleNamespace() {
    return podNames["my-component-name"];
  }
}

Then you can use <div class={{this.styleNamespace}}> in your template.hbs.

I think it's related to: https://github.com/ebryn/ember-component-css/issues/335

erkie avatar Jan 03 '20 22:01 erkie

yea, so, the way i have this working in the beta, is to wrap the contents from the template that has a corresponding style file in a let block that defines a styleNamespace variable.

https://github.com/ebryn/ember-component-css/blob/aa2ce7a5aeea65fefef7d5096dda4a051e8ce3a6/lib/scope-templates.js#L18

but then, for non template only components you’d have “this.styleNamespace”, but for template only you’d have to use the “styleNamespace”.. which is one of the weird ergonomics i don’t love about the beta.

It is something that could be easily back ported though.. what’s your all’s opinion? You think of a better way you’d rather use it? It’s just, cause there’s no backing component class, there’s no real “this” in the same sense.. and since it shouldn’t be something that gets passed in, nor is it dynamic, it shouldn’t be an @ variable. All that’s left is a “ghost” local variable that gets defined for you. but that feels weird still..

webark avatar Jan 04 '20 14:01 webark

Will ember-modilers fit your needs?

Modifier app/modifiers/style-namespace

import { setModifierManager } from '@ember/modifier';
import podNames from 'ember-component-css/pod-names';

export default setModifierManager(
  () => ({
    createModifier() {},
    installModifier(_state, element, args) {
      const [componentOrPath] = args.positional;

      if (typeof componentOrPath == "string") {
        const className = podNames[componentOrPath]
        element.classList.add(className)
      } else {
        // we can pass "this" into modifier but I didn't find a good way to automaticaly find out component's path
      }
    },
    updateModifier() {},
    destroyModifier() {},
  }),
  class StyleNamespaceModifier {}
);

<div {{style-namspace "images/gallery"}}>... </div>

fastindian84 avatar Jan 13 '20 00:01 fastindian84

@fastindian84 Yea, i thought about modifiers, but needing to add in a modifier and pass on the path that you are at, your just setting the class name again anyway.

This addon really only needs to do 2 simple things. Take the path of a style file and a template, and provide a variable that is shared in both, and unique to that combo. The concatenation all of those style files together into one style file can (and hopefully one day will be) be separated into its package.

The big issue is that how do you provide a new variable to one of these templates, when they are essentially designed to be independent functions. I haven’t found a solution that has felt satisfying, so it’s left me feeling somewhat stuck, and if you are going to need to be explicit about it anyway, then might as well ditch this part and just do..

<div class=“class-we-made-up”> .class-we-made-up {

webark avatar Jan 15 '20 12:01 webark

I've made an ugly hack to make it work with glimmer components. Template only components are not supported. Maybe this helps someone?

// instance-initializers/fix-ember-component-css-style-namespace.ts

import ApplicationInstance from "@ember/application/instance";
// @ts-ignore
import podNames from "ember-component-css/pod-names";

function initialize(appInstance: ApplicationInstance): void {
    for (const [componentPath, styleNamespace] of Object.entries(podNames as Record<string, string>)) {
        // https://api.emberjs.com/ember/3.14/classes/ApplicationInstance/methods/factoryFor?anchor=factoryFor
        const factoryForComponent = appInstance.factoryFor(`component:${componentPath}`); // Instance ist FactoryManager
        if (factoryForComponent === undefined) {
            continue; // no component
        }

        const componentCtor = factoryForComponent.class;
        if (componentCtor === undefined) {
            continue;
        }

        // https://github.com/ebryn/ember-component-css/issues/293#issuecomment-483425075
        Object.defineProperty(componentCtor.prototype, "styleNamespace", {
            configurable: true,
            enumerable: true,
            get: function () {
                return styleNamespace;
            }
        });
    }
}

export default {
    after: "route-styles", // run after `ember-component-css` "route-styles"-Initializer
    initialize
}

mschorsch avatar Jan 16 '20 15:01 mschorsch

Nice idea, but might not work with engines?

BryanCrotaz avatar Jan 16 '20 15:01 BryanCrotaz

that’s not a bad idea. I think it would. @mschorsch you mind putting that into a PR, and we can tweek it a little and get it merged in?

webark avatar Jan 16 '20 15:01 webark

but yea.. still doesn’t work with template only.

webark avatar Jan 16 '20 15:01 webark

@mschorsch tgats what i’m doing in the “registry way” also..

https://github.com/ebryn/ember-component-css/blob/aa2ce7a5aeea65fefef7d5096dda4a051e8ce3a6/addon/utils/add-component-style-namespace.js#L2-L21

webark avatar Jan 16 '20 15:01 webark

Any movement on 'productizing' one of these solutions?

adambedford avatar Mar 11 '20 00:03 adambedford

I have been away from this add-on since before octane dropped, and it changed a number of internals so some things no longer work as expected. The earliest I could look at this would be next week, and I will try to do so.

webark avatar Mar 27 '20 08:03 webark

what's the downside to enforcing that a user has to create a backing component file to use this? If all that's required is creating a blank js file then I don't see much downside, especially if it let's us move forward with at least some glimmer support right now...

erichaus avatar Apr 12 '20 20:04 erichaus

I spent some time finalizing the "registry-way" branch of this, and am looking to releasing a beta this coming week.

webark avatar Jul 11 '20 05:07 webark

"ember-component-css": "^0.6.9", currently does not support template-only feature in octane.

Previously, we didn't need to create empty component.js file, but now after upgraded to Ember 3.16 with the following feature flag turned on.

  "template-only-glimmer-components": true

In order to get styles applied to component, we have to create empty component.js

import Component from '@ember/component';

export default Component.extend({});

Not sure if this is an octane issue or ember-component-css issue.

Also, curious if this PR would fix it? @webark

Thank you

PS: I felt this is similar to an issue from ember-css-modules

adong avatar Aug 06 '20 05:08 adong

This is an issue with this addon.

there is a new release that is being worked on. An alpha version has been released.

We'll be creating a separate addon to support legacy versions of this addon. (the breaking changes with an additional legacy addon would address are listed in #333)

This PR might solve your issues (and you can point to that fork), I just have limited time that i can invest, and so i have been putting any time i have into getting the new version ready so that we have long term stability and success, rather then this option which is fundamentally flawed and not supported long term.

webark avatar Aug 06 '20 05:08 webark

Just to note here as well. ember-component-css is kind-of deprecated in favor of ember-cli-styles which works great on Ember 4 (and Glimmer components)! Thanks to @webark for all his hard work on it!

boris-petrov avatar Mar 23 '22 12:03 boris-petrov

Fabulous, but there's no readme at https://github.com/webark/ember-cli-styles so no idea how to use it...

BryanCrotaz avatar Mar 23 '22 12:03 BryanCrotaz

Yeah, I've outlined the changes I did here. PRs are welcome! :)

boris-petrov avatar Mar 23 '22 12:03 boris-petrov