webark / ember-component-css

An Ember CLI addon which allows you to specify styles for individual components
MIT License
542 stars 115 forks source link

Doesn't work in Octane (3.15) glimmer component #342

Open BryanCrotaz opened 4 years ago

BryanCrotaz commented 4 years ago

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

erkie commented 4 years ago

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

webark commented 4 years ago

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..

fastindian84 commented 4 years ago

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>
webark commented 4 years ago

@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 {

mschorsch commented 4 years ago

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
}
BryanCrotaz commented 4 years ago

Nice idea, but might not work with engines?

webark commented 4 years ago

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 commented 4 years ago

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

webark commented 4 years ago

@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

adambedford commented 4 years ago

Any movement on 'productizing' one of these solutions?

webark commented 4 years ago

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.

hoIIer commented 4 years ago

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...

webark commented 4 years ago

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

adong commented 3 years ago

"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

webark commented 3 years ago

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.

boris-petrov commented 2 years ago

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!

BryanCrotaz commented 2 years ago

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

boris-petrov commented 2 years ago

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