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

SVG element parsing regression for svg element tag names with camelCase: ReferenceError: radialGradient is not defined when converting to gts-component #1550

Closed
johanrd opened this issue Jan 24, 2024 · 8 comments · Fixed by emberjs/babel-plugin-ember-template-compilation#33

Comments

@johanrd
Copy link

johanrd commented Jan 24, 2024

I get a regression when converting a template-only-hbs-component into a template-only-gts-component.

The gts-parser seems to think that the svg element radialGradient is a glimmer component, and not an svg element:

Error in browser console:

radial-gradient-reproduction.ts:1 Uncaught (in promise) ReferenceError: radialGradient is not defined
  at Object.scope (radial-gradient.ts:1:956)
  at meta (opcode-compiler.js:632:1)

Reproduction:

// radial-gradient-reproduction.gts:

const RadialGradientReproduction = <template>
  <svg viewBox="0 0 10 10" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
    <defs>
      <radialGradient id="myGradient">
        <stop offset="10%" stop-color="gold" />
        <stop offset="95%" stop-color="red" />
      </radialGradient>
    </defs>
    <circle cx="5" cy="5" r="4" fill="url('#myGradient')" />
  </svg>
</template>;

export default RadialGradientReproduction;

Environment:

"@glimmer/component": "^1.1.2",
"@embroider/compat": "3.4.3",
"@embroider/core": "3.4.3",
"@embroider/router": "2.1.6",
"@embroider/webpack": "3.2.1",
"ember-template-imports": "^4.0.0",
"ember-source": "5.5.0",
@johanrd johanrd changed the title SVG element parsing regression: ReferenceError: radialGradient is not defined when conveting to gts-component SVG element parsing regression: ReferenceError: radialGradient is not defined when converting to gts-component Jan 24, 2024
@NullVoxPopuli
Copy link
Contributor

Do you happen to have what the compiled version of that component looks like?

@johanrd
Copy link
Author

johanrd commented Jan 25, 2024

@NullVoxPopuli like this?

radial-gradient-reproduction.ts:

import { template } from "@ember/template-compiler";
const RadialGradientReproduction = template(`
  <svg viewBox="0 0 10 10" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
    <defs>
      <radialGradient id="myGradient">
        <stop offset="10%" stop-color="gold" />
        <stop offset="95%" stop-color="red" />
      </radialGradient>
    </defs>
    <circle cx="5" cy="5" r="4" fill="url('#myGradient')" />
  </svg>
`, {
    eval () {
        return eval(arguments[0]);
    }
});
export default RadialGradientReproduction;

(how the browser loads the component)

@johanrd johanrd changed the title SVG element parsing regression: ReferenceError: radialGradient is not defined when converting to gts-component SVG element parsing regression for svg element tag names with camelCase: ReferenceError: radialGradient is not defined when converting to gts-component Jan 25, 2024
@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Feb 29, 2024

@NullVoxPopuli
Copy link
Contributor

This has all the tags we'd want: https://www.npmjs.com/package/html-spec-tags?activeTab=readme

@NullVoxPopuli
Copy link
Contributor

or, maybe this is an issue with getTemplateLocals (what we use to build the scope argument in strict mode):

image

@chancancode
Copy link
Contributor

I ran into this too.

I didn't try super hard to break it, so this is not necessarily conclusive, but I added a test to the the strict mode tests and it passes:

  @test
  'camelCased SVG elements are not considered identifiers/references'() {
    // https://developer.mozilla.org/en-US/docs/Web/SVG/Element/linearGradient
    const Foo = defineComponent({}, stripTight`
      <svg viewBox="0 0 10 10">
        <defs>
          <linearGradient id="myGradient">
            <stop offset="5%" stop-color="gold" />
            <stop offset="95%" stop-color="red" />
          </linearGradient>
        </defs>

        <circle cx="5" cy="5" r="4" fill="url('#myGradient')" />
      </svg>
    `);

    this.renderComponent(Foo);
    this.assertHTML(stripTight`
      <svg viewBox="0 0 10 10">
        <defs>
          <linearGradient id="myGradient">
            <stop offset="5%" stop-color="gold" />
            <stop offset="95%" stop-color="red" />
          </linearGradient>
        </defs>

        <circle cx="5" cy="5" r="4" fill="url('#myGradient')" />
      </svg>
    `);
    this.assertStableRerender();
  }

So that matches my suspicion that the problem is not, strictly speaking, in glimmer-vm. I think the problem is likely that babel-plugin-ember-template-compilation is using getTemplateLocals() and that function is no good and fundamentally unsound.

@chancancode
Copy link
Contributor

chancancode commented Mar 12, 2024

While waiting for this to be fixed, I made a few template-only components for the few SVG elements that has this problem, so I can import them into .gts files and unblock the conversion:

Screen Shot 2024-03-11 at 11 49 39 PM

(If you aren't using TypeScript you don't need a .ts/.js file for this)

On the far right is a .gts component with import clipPath from "my-app/components/svg/clip-path";.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants