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

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

Open
BryanCrotaz opened this issue Jan 3, 2020 · 18 comments
Open

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

BryanCrotaz opened this issue Jan 3, 2020 · 18 comments

Comments

@BryanCrotaz
Copy link

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

@erkie
Copy link

erkie commented Jan 3, 2020

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: #335

@webark
Copy link
Owner

webark commented Jan 4, 2020

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
Copy link

fastindian84 commented Jan 13, 2020

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
Copy link
Owner

webark commented Jan 15, 2020

@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
Copy link

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
Copy link
Author

Nice idea, but might not work with engines?

@webark
Copy link
Owner

webark commented Jan 16, 2020

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
Copy link
Owner

webark commented Jan 16, 2020

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

@webark
Copy link
Owner

webark commented Jan 16, 2020

@adambedford
Copy link

Any movement on 'productizing' one of these solutions?

@webark
Copy link
Owner

webark commented Mar 27, 2020

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
Copy link

hoIIer commented Apr 12, 2020

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
Copy link
Owner

webark commented Jul 11, 2020

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

@adong
Copy link

adong commented Aug 6, 2020

"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
Copy link
Owner

webark commented Aug 6, 2020

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
Copy link

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
Copy link
Author

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

@boris-petrov
Copy link

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

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

No branches or pull requests

9 participants