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: add wireit #22

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

chore: add wireit #22

wants to merge 2 commits into from

Conversation

vdegenne
Copy link

Just for the form I added wireit if you are interested, sources are already compiled using typescript so web-test-runner doesn't need to use rely on esbuild, so I removed the dependency.
There is also no need for rimraf, wireit automatically cleans.

@43081j
Copy link
Owner

43081j commented Oct 22, 2023

sorry for the huge delay, just getting to this now

i'd like to keep the esbuild plugin i think, just to keep sourcemaps simple in WTR (i've had trouble before with them when using build output as the test files)

but otherwise looks good, am up for using wireit for sure

@vdegenne
Copy link
Author

vdegenne commented Oct 24, 2023

Do you want me to just put the esbuild plugin block back?

@43081j
Copy link
Owner

43081j commented Oct 24, 2023

if you don't mind, yes please. it will keep things clean i think

looks good to me otherwise

@vdegenne
Copy link
Author

if you don't mind, yes please. it will keep things clean i think

looks good to me otherwise

But I think this PR is not needed then, the whole point of using wireit here was to break the build and test processes into 2 separate steps, to remove @web/dev-server-esbuild and rimraf dependencies and to be able to use npm run test --watch for fast testing during development.
If I add esbuildPlugin back it doesn't make sense, that means the code will unnecessarily be compiled twice in the wireit cascade.
You could just ignore this change or accept it and find a workaround if you ever bump into the issue you were mentioning (which I don't have on my end). I just think wireit makes things better overall.

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.

None yet

2 participants