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

Move all depenendencies to dev deps #764

Merged
merged 3 commits into from
Jun 13, 2023
Merged

Conversation

tmeasday
Copy link
Member

@tmeasday tmeasday commented Jun 13, 2023

Fixes #762

I'm not sure why these utilities were in deps, no dev deps. I think that TSUP will never bundle deps, whereas maybe previously webpack would happily bundle everything? Not sure.

@tmeasday tmeasday merged commit 1fc8487 into main Jun 13, 2023
29 of 32 checks passed
@tmeasday tmeasday deleted the tom/move-csf-tools-to-dev-deps branch June 13, 2023 05:00
@tmeasday
Copy link
Member Author

Going to hard merge this one to fix the action (confirmed on canary).

@tmeasday tmeasday mentioned this pull request Jun 13, 2023
@mattseddon
Copy link

mattseddon commented Jun 13, 2023

See https://tsup.egoist.dev/#excluding-packages (I think you needed to add the --noExternal flag).

@tmeasday
Copy link
Member Author

That's a good move if we want to revert this PR. Will seek clarification as to whether that's the right direction from others.

@ndelangen
Copy link
Member

@tmeasday so the action absolutely needs everything bundled in, because it has no install step (for performance reasons).

The node-api might be different, because you might want to share dependencies.

Having them all bundled in might not be a bad situation though, this PR seems fine as is for the foreseeable future.

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.

Broken Action
3 participants