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 esbuild to the NPM @esbuild scope #53

Merged
merged 4 commits into from Dec 9, 2022
Merged

Move esbuild to the NPM @esbuild scope #53

merged 4 commits into from Dec 9, 2022

Conversation

cw789
Copy link
Contributor

@cw789 cw789 commented Sep 5, 2022

This is my take on #52.
It's not ready until the new breaking version number of esbuild is known for sure.

Update: Ready now → esbuild v0.16.0

@josevalim
Copy link
Member

Looks great! A couple notes:

  1. Let's bump the esbuild version once the release is out
  2. Let's update the manual install instructions in Esbuild moduledoc :)

@cw789
Copy link
Contributor Author

cw789 commented Sep 7, 2022

  1. Totally agree - will do
  2. Good point - will do

Found one more issue with my solution.
target() is used in bin_path()https://github.com/phoenixframework/esbuild/blob/main/lib/esbuild.ex#L128
Means nearly everyone needs to reinstall via mix esbuild.install...

I could additionally do a Version.compare/2 in bin_path() or one in target() and move legacy targets to the target() function.

@josevalim
Copy link
Member

Means nearly everyone needs to reinstall via mix esbuild.install...

Well, they will have to reinstall once they change the version or update this package with a new version, so that's ok. :)

@cw789
Copy link
Contributor Author

cw789 commented Sep 7, 2022

Right, sounds good. Thank you.

@cw789 cw789 marked this pull request as ready for review December 7, 2022 12:01
@chrismccord chrismccord merged commit 9495b9a into phoenixframework:main Dec 9, 2022
@chrismccord
Copy link
Member

❤️❤️❤️🐥🔥

@cw789 cw789 deleted the esbuild_scope branch December 9, 2022 16:18
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

3 participants