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

Switch to esbuild for faster builds and also to allow using more modern JavaScript syntax and features #6909

Merged
merged 112 commits into from
Sep 12, 2024

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Feb 28, 2024

@plotly/plotly_js

TODOs:

  • Fix regl_codegen
  • Fix custom bundles
  • Test new bundle in amdefine
  • Test new bundle in requirejs
  • Test new bundle in kaleido
  • Test new bundle in orca
  • additional QA on the python side on loading the bundle

@archmoj
Copy link
Contributor Author

archmoj commented Aug 22, 2024

@alexcjohnson @gvwilson
This PR is ready to go 🚀

@gvwilson gvwilson requested a review from marthacryan August 22, 2024 13:53
@gvwilson
Copy link
Contributor

thank you - @marthacryan can you please have a look at this as well?

@archmoj archmoj requested a review from alexcjohnson August 22, 2024 15:10
CUSTOM_BUNDLE.md Outdated Show resolved Hide resolved
@archmoj
Copy link
Contributor Author

archmoj commented Aug 23, 2024

I'm looking forward to @alexcjohnson's review on this as well. Also thanks to @marthacryan and @birkskyum reviews & contributions 🙏

This PR helps unblock Chart2Music keyboard and sound accessibility features in #6680. cc: @Coding-with-Adam

It might be a good idea and timing to include this in the upcoming RC release of [v2.35.0] (https://github.com/plotly/plotly.js/milestone/72) and our QA. cc: @gvwilson @ndrezn @LiamConnors

It would also help our ongoing and future development and testing of other pull requests. cc: @emilykl @antoinerg

@gvwilson
Copy link
Contributor

I approve. Let's hold off on the merge until next week (feels too big to do on a Friday), but congratulations everyone - well done.

@ndrezn
Copy link
Member

ndrezn commented Aug 23, 2024

Super excited about this! This change will really improve the development experience for Plotly.js. Kudos 🥳

@Coding-with-Adam
Copy link
Contributor

Wonderful news 🚀 . Thank you @archmoj; Thank you @marthacryan

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

💃 I'm super excited for this. Looks great!

@archmoj
Copy link
Contributor Author

archmoj commented Aug 23, 2024

Thanks for the reviews!
Before merging this PR, I'd like to do additional QA on the python side on loading the bundle.

@archmoj archmoj modified the milestones: v2.35.0, v3.0.0 Aug 27, 2024
@birkskyum
Copy link
Contributor

birkskyum commented Sep 6, 2024

We'll need to configure esbuild to also allow require(file.css) because of:

One option is here (esbuild-style-plugin), but there might be built-in support for this in esbuild:

birkskyum and others added 4 commits September 6, 2024 21:52
 - Resolved conflicts in package-lock.json by copying lock file from master then npm install
@archmoj archmoj merged commit bb2b8e6 into master Sep 12, 2024
1 check passed
@archmoj archmoj deleted the build-with-esbuild branch September 12, 2024 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature something new infrastructure build process etc. P1 needed for current cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants