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

Astro 4 update and build fix #11

Merged
merged 3 commits into from
Mar 21, 2024
Merged

Astro 4 update and build fix #11

merged 3 commits into from
Mar 21, 2024

Conversation

revgum
Copy link
Contributor

@revgum revgum commented Jan 20, 2024

The update to Astro 4 and the npm run build seems to work without issue in this update.

I see that the npm run test is broken on the main branch and remains broken on this PR. The test/helpers.ts has a hardcoded reference to astro.js which doesn't exist. I imagine this was part of some CI configuration that the previous team had setup and just hasn't been translated over to this repo.

@revgum
Copy link
Contributor Author

revgum commented Jan 20, 2024

I did use yalc to publish and link to a local build of this package for testing, and it looks like Astro 4 was running.

Screenshot 2024-01-20 at 5 34 38 AM

On the other hand, npm run build with this linked package fails due to astro check having problems and finally astro build can't find the Sharp library for image assets.

I'm happy to do some more testing if you like, but I think for the time being I'll just stick with the older version.

@Ayfri
Copy link

Ayfri commented Mar 12, 2024

In my opinion you should publish this as a separate package, I think they will never update this package again.

Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@hashrock hashrock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hashrock hashrock merged commit 7fc1a15 into denoland:main Mar 21, 2024
1 check passed
@Ayfri
Copy link

Ayfri commented Mar 21, 2024

Oh finally, thanks !

@kt3k
Copy link
Member

kt3k commented Mar 21, 2024

Hi @matthewp

We'd like to publish this change to @astrojs/deno to make it up-to-date with Astro 4. Can we (Deno team) have publish access to that npm package? Or should we publish it as a different package?

@matthewp
Copy link
Contributor

Hey @kt3k, sorry I missed the comment! We'd like to keep the @astrojs org to Astro core maintained packages, so I would recommend creating a new package for you to publish to.

@kt3k
Copy link
Member

kt3k commented Apr 16, 2024

Thanks for your reply @matthewp! Ok. Got it. We'll publish it under a different name.

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.

5 participants