Skip to content

Conversation

@camdecoster
Copy link
Contributor

@camdecoster camdecoster commented Oct 15, 2025

Description

Add build step before generating the schema.

Closes #7538.

Changes

  • Lints/formats files
  • Consolidates esbuild configs to one file
  • Updates references per above change
  • Adds build step before generating the schema via npm run schema

Testing

  • Check CI to see if it's completing successfully
  • Be on master
  • Do not start the dashboard
  • Make a change to a description in an attributes file
  • Run npm run schema
  • Note that test/plot-schema.json is not updated
  • Switch to this branch
  • Run npm run schema
  • Note that test/plot-schema.json IS updated

Notes

  • Running the dashboard with npm start builds the library every time a change is made
  • If the schema is generated when the dashboard is running, the changes to the attributes are picked up
  • When the dashboard isn't running, the changes aren't picked up
  • The build script takes care of this case and uses the same esbuild config as the dashboard

@camdecoster camdecoster marked this pull request as ready for review October 15, 2025 22:52
@camdecoster camdecoster self-assigned this Oct 15, 2025
logLevel: 'info'
};

const devtoolsConfig = {
Copy link
Contributor

@emilykl emilykl Oct 16, 2025

Choose a reason for hiding this comment

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

@camdecoster Could you add a comment above each of these configs explaining what it's used for (to the extent that you know)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll come up with something, but the variable names were my attempt at a description.

Copy link
Contributor

Choose a reason for hiding this comment

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

@camdecoster After the cleanup you've done, does this step really need a separate file? Couldn't you just now add build(localDevConfig); to the beginning of the makeSchema() function in schema.mjs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a fair question. I envisioned this script as being one that could be called elsewhere, but that seems like it violates the YAGNI principle. I'll do as you suggest. We can always add it back.

@emilykl
Copy link
Contributor

emilykl commented Oct 16, 2025

@camdecoster Not a big deal but looks like this results in an extra plotly.js build at the end of npm run build:

(base) ekl@Emilys-MacBook-Air-2 plotly.js % npm run build

> plotly.js@3.1.1 build
> npm run empty-dist && npm run preprocess && npm run find-strings && npm run bundle && npm run extra-bundles && npm run locales && npm run schema dist && npm run stats

> plotly.js@3.1.1 empty-dist
> node tasks/empty_dist.js

empty /Users/ekl/code/plotly.js/dist/topojson/
empty /Users/ekl/code/plotly.js/dist/

> plotly.js@3.1.1 preprocess
> node tasks/preprocess.js

> plotly.js@3.1.1 find-strings
> node tasks/find_locale_strings.js

ok find_locale_strings - wrote new key file.

> plotly.js@3.1.1 bundle
> node tasks/bundle.mjs

  dist/plotly.js  10.7mb ⚠️

⚡ Done in 732ms

 ...
 ...
 ...

ok plotly-locale-bs.js
ok plotly-locale-af.js
ok plotly-locale-es-pe.js

> plotly.js@3.1.1 schema
> node devtools/test_dashboard/build.mjs && node tasks/schema.mjs dist

  build/plotly.js  11.0mb ⚠️

⚡ Done in 225ms
ok plot-schema.json

> plotly.js@3.1.1 stats
> node tasks/stats.js

Copy link
Contributor

@emilykl emilykl left a comment

Choose a reason for hiding this comment

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

thx @camdecoster

@camdecoster
Copy link
Contributor Author

@camdecoster Not a big deal but looks like this results in an extra plotly.js build at the end of npm run build

@emilykl this shouldn't cause issues because it's building the library in two different locations: dist and build. The one in build is what's used to generate the schema. It's a little extra overhead, but now we don't have to worry about the schema getting out of sync with the library.

@camdecoster camdecoster merged commit 80c7c7d into master Oct 20, 2025
5 of 6 checks passed
@camdecoster camdecoster deleted the cam/7538/build-bundle-before-schema branch October 20, 2025 20:28
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.

plotly.js bundle isn't rebuilt when generating schema

2 participants