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

chore: dart-sass, glimmer/component v2 #939

Closed
wants to merge 3 commits into from

Conversation

Techn1x
Copy link
Contributor

@Techn1x Techn1x commented Nov 11, 2024

  • swap from node-sass to sass (aka dart-sass)
    • node-sass is long deprecated, and I cannot install it on my machine, so I had no choice.
  • updated lockfile version (ran pnpm install with v9.5.0) + updated CI to use pnpm 9
  • update to @glimmer/component v2 and allow it in peer deps.

Not too concerned about the sass change, since it's a build / devdependency. I ran pnpm build and it produced the same vendor output (see review comment)

The glimmer/component v2 update just drops ember 3.13 and makes it a v2 addon.
https://github.com/emberjs/ember.js/blob/main/packages/%40glimmer/component/CHANGELOG.md

Given the above, I think this is safe to publish as a patch or minor


I couldn't do the glimmer/component update without first ditching node-sass. If you feel it should be separated, feel free to do so, but I felt they are both small enough changes.

@Techn1x
Copy link
Contributor Author

Techn1x commented Nov 11, 2024

Can't seem to get past this error :(

Build Error (WebpackBundler)

ember-prism tried to import "@glimmer/component" in "ember-prism/components/code-block.js" from addon code, but "@glimmer/component" is a devDependency. You may need to move it into dependencies.

ember-prism looks deprecated/archived

I tried adding this to root package.json, with no luck

"pnpm": {
    "packageExtensions": {
      "ember-prism": {
        "dependencies": {
          "@glimmer/component": "*"
        }
      }
    }
  }

Any ideas? @cibernox @mkszepp

@Techn1x
Copy link
Contributor Author

Techn1x commented Nov 11, 2024

Related ember-power-calendar PR cibernox/ember-power-calendar#527

@mkszepp
Copy link
Collaborator

mkszepp commented Nov 11, 2024

@Techn1x thank for your PR... i will take a look today or next days, but i think we need to remove this package before we can land glimmer v2 support

@Techn1x
Copy link
Contributor Author

Techn1x commented Nov 11, 2024

Thanks! Let me know if you would like this PR to ditch the glimmer/component v2 changes for now and we can get the sass / pnpm changes merged to start with

@mkszepp
Copy link
Collaborator

mkszepp commented Nov 11, 2024

@Techn1x the issue with node-sass i will close with this PR #938

@Techn1x
Copy link
Contributor Author

Techn1x commented Nov 11, 2024

Oooh even better!

@mkszepp
Copy link
Collaborator

mkszepp commented Nov 11, 2024

lets see if it works after this changes... #941

@Techn1x
Copy link
Contributor Author

Techn1x commented Nov 11, 2024

I'm in the opposite timezone, I won't be at my computer for another 12 hours or so - you're welcome to change/close this PR if you like, otherwise I can remake the glimmer/component V2 update tomorrow

@mkszepp
Copy link
Collaborator

mkszepp commented Nov 11, 2024

looks like we can't get a green ci atm, because they are using @ember/owner and this was introduced after 4.8 🙈... startet a discusson on discord channel... https://discord.com/channels/480462759797063690/485447409296736276/1305502264479191132

@Techn1x
Copy link
Contributor Author

Techn1x commented Nov 12, 2024

Thanks for sluething that. CI tests prevail! Sounds like they might need an ember-try config over at glimmer/component

Given the meaningful changes in this PR have been merged and/or extracted elsewhere, going to close this one

@Techn1x Techn1x closed this Nov 12, 2024
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

Successfully merging this pull request may close these issues.

2 participants