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

Remove unnecessary transform for content imports #6808

Closed
wants to merge 1 commit into from

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Apr 10, 2023

Changes

In vite-plugin-content-imports, there's a transform to escape Vite references. However, in the load hook we already did that, so we should be able to remove the transform hook.

Fix #6058 (there was a sourcemap warning in the transform hook)

Testing

Existing tests should pass.

Docs

n/a. bug fix.

@changeset-bot
Copy link

changeset-bot bot commented Apr 10, 2023

🦋 Changeset detected

Latest commit: dfc898e

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Apr 10, 2023
@bluwy bluwy requested a review from bholmesdev April 10, 2023 09:53
@bluwy bluwy marked this pull request as draft April 10, 2023 14:05
@bluwy
Copy link
Member Author

bluwy commented Apr 10, 2023

Looks like this break docs 👀

@bluwy bluwy removed the request for review from bholmesdev April 10, 2023 14:06
@bholmesdev
Copy link
Contributor

Thanks @bluwy! Was actually refactoring this while working on data collections, and I found a better fix: don't use load at all. Always use the transform hook to handle flags like this to avoid Vite post-processing for certain files. I was finding that Vite's JSON transform was super aggressive even on flag imports and virtual modules. Moving everything to a transform, I was able to avoid this and keep our tests passing. Think I'll make a code refactor PR so we don't need to wait on data collections for the change.

@bluwy
Copy link
Member Author

bluwy commented Apr 10, 2023

Awesome! That makes sense to me, and thanks for chiming in. I'll close this for now then so we can fix it in a different way.

@bluwy bluwy closed this Apr 10, 2023
@bluwy bluwy deleted the remove-transform-content-import branch April 10, 2023 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Astro 2.0 warning when using sourcemaps
2 participants