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.

Sign up for GitHub

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

Jump to bottom

Replace custom loader with @node-loaders/tsx #898

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Replace custom loader with @node-loaders/tsx #898

wants to merge 1 commit into from

Conversation

remcohaszing
Copy link
Member

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and discussions and couldn’t find anything or linked relevant results below
  • I made sure the docs are up to date
  • I included tests (or that’s not needed)

Description of changes

We need a loader to load our JSX test file. We currently use a local script, but there are several packages that do this. We don’t need to maintain our own.

This replaces our custom loader with @nodejs-loaders/tsx by the Node.js team.

@remcohaszing
Replace custom loader with @node-loaders/tsx
59aba31 
We need a loader to load our JSX test file. We currently use a local
script, but there are several packages that do this. We don’t need to
maintain our own.

This replaces our custom loader with `@nodejs-loaders/tsx` by the
Node.js team.
@remcohaszing remcohaszing added 🏡 area/internal This affects the hidden internals 🕸️ area/tests This affects tests 👶 semver/patch This is a backwards-compatible fix 🦋 type/enhancement This is great to have 🤞 phase/open Post is being triaged manually labels Mar 6, 2025
@remcohaszing
Copy link
Member Author

Apparently this doesn’t work with all Node.js versions we support.

ChristianMurphy
ChristianMurphy approved these changes Mar 6, 2025
View reviewed changes
Copy link
Member

@ChristianMurphy ChristianMurphy left a comment

Choose a reason for hiding this comment

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

Looks good to me, let's merge this when hydrogen is supported, or when we drop hydrogen support

@ChristianMurphy ChristianMurphy added the 🧑 semver/major This is a change label Mar 6, 2025
@github-actions github-actions bot removed the 👶 semver/patch This is a backwards-compatible fix label Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChristianMurphy ChristianMurphy ChristianMurphy approved these changes

Assignees
No one assigned
Labels
🏡 area/internal This affects the hidden internals 🕸️ area/tests This affects tests 🤞 phase/open Post is being triaged manually 🧑 semver/major This is a change 🦋 type/enhancement This is great to have
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@remcohaszing @ChristianMurphy