glint icon indicating copy to clipboard operation
glint copied to clipboard

use new glimmer ElementNode sub nodes

Open patricklx opened this issue 1 year ago • 14 comments

there are now also following nodes:

  • name node: ElementNameNode
  • start tag node: ElementStartNode

example:

/*
  <Foo.bar.x attr='2'></Foo.bar.x>
   ^-- ElementPartNode
       ^-- ElementPartNode
          ^- ElementPartNode
   ^-------- ElementNameNode
  ^------------------ ElementStartNode
                      ^----------- ElementEndNode
 */

afterwards completions/validations can be add for attributes and element tags In #663

patricklx avatar Jan 29 '24 14:01 patricklx

@NullVoxPopuli any idea how I can fix this:

Error: node_modules/@glimmer/interfaces/index.d.ts(1,2[9](https://github.com/typed-ember/glint/actions/runs/7698348329/job/20977417907?pr=696#step:5:10)): error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.
Error: node_modules/@glimmer/interfaces/index.d.ts(3,15): error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.
Error: node_modules/@glimmer/interfaces/index.d.ts(4,15): error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.
Error: node_modules/@glimmer/interfaces/index.d.ts(5,15): error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.
Error: node_modules/@glimmer/interfaces/index.d.ts(6,15): error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.
Error: node_modules/@glimmer/interfaces/index.d.ts(7,15): error TS2834: Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Consider adding an extension to the import path.

patricklx avatar Jan 30 '24 10:01 patricklx

Does skipLibCheck help?

Otherwise, glimmer has more bugs we need to fix (all relative imports should always use extensions when using node16, so it shouldn't be too bad of a fix)

NullVoxPopuli avatar Jan 30 '24 12:01 NullVoxPopuli

Does skipLibCheck help?

Otherwise, glimmer has more bugs we need to fix (all relative imports should always use extensions when using node16, so it shouldn't be too bad of a fix)

yes, it helped!

patricklx avatar Jan 30 '24 13:01 patricklx

@patricklx would there be any noticeable editor behavior changes with this PR?

(or is it "just the same, but using the new parser capabilities"?)

NullVoxPopuli avatar Jan 30 '24 15:01 NullVoxPopuli

i'm not seeing and difference

patricklx avatar Jan 30 '24 15:01 patricklx

@patricklx new versions published, can you try the .1 release?

NullVoxPopuli avatar Feb 01 '24 00:02 NullVoxPopuli

nope, some new errors-...

Error: node_modules/@types/ember__helper/index.d.ts(3,81): error TS147[9](https://github.com/typed-ember/glint/actions/runs/7739973176/job/21103924609?pr=696#step:5:10): The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("@glimmer/manager")' call instead.
Error: node_modules/@types/ember__helper/index.d.ts(4,53): error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("@glimmer/runtime")' call instead.
Error: node_modules/@glimmer/modifier/dist/commonjs/index.d.ts(1,20): error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("@glimmer/runtime")' call instead.

https://github.com/microsoft/TypeScript/issues/52529 https://github.com/microsoft/TypeScript/pull/53426

looks like @ef4 knows something about this

patricklx avatar Feb 01 '24 10:02 patricklx

ah yeah, we're probably blocked on ember-source needing to actually ship as a type=module package.

NullVoxPopuli avatar Feb 01 '24 15:02 NullVoxPopuli

@NullVoxPopuli the error is under @types folder. So we can make that type: module? https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/ember__helper/package.json

And here? https://github.com/patricklx/glimmer.js/blob/master/packages/%40glimmer/modifier/package.json

patricklx avatar Feb 01 '24 16:02 patricklx

does changing all of @types/ember* to type=module work locally? :thinking:

NullVoxPopuli avatar Feb 01 '24 22:02 NullVoxPopuli

does changing all of @types/ember* to type=module work locally? 🤔

it fixes some errors, but also needed to fix @glimmerx/modifier and @glimmer/modifier.

still, at the end it says node_modules/@types/ember__helper/index.d.ts:1:24 - error TS2307: Cannot find module 'ember/-private/type-utils' or its corresponding type declarations.

patricklx avatar Feb 02 '24 08:02 patricklx

Ahh, yeah, probably because all the private relative imports need extensions at that point. Hmm

NullVoxPopuli avatar Feb 02 '24 12:02 NullVoxPopuli

I've added skipLibCheck back.

It doesn't seem we can build without it.

NullVoxPopuli avatar Feb 02 '24 17:02 NullVoxPopuli

@patricklx FYI 0.89.0 is out with https://github.com/glimmerjs/glimmer-vm/pull/1568 The information you need should now be in ElementNode.{path,openTag,closeTag}: https://github.com/glimmerjs/glimmer-vm/pull/1568/commits/68509ace73a77d1b36eab9c958006c9f1c98ec06#diff-21f1f3fd9aaae780472e1cf9cf079d1a05efd0fd642f6758c963dc7e969a4b30

Should also fix #706

chancancode avatar Mar 11 '24 18:03 chancancode