Change version injection to be back in a package#2322
Conversation
|
Size Change: -4.4 kB (-0.6%) Total Size: 732 kB
ℹ️ View Unchanged
|
| }, | ||
| "devDependencies": { | ||
| "@khanacademy/pure-markdown": "workspace:*", | ||
| "perseus-build-settings": "workspace:*", |
There was a problem hiding this comment.
This was missing the build-settings dep, which it should have!
handeyeco
left a comment
There was a problem hiding this comment.
Thanks for looking into this! Couple of thoughts:
- The bundle sizes seemed to get bigger (
perseus-editorgrew 3MB?) which makes me think there's a bug here - I worry about the long-term maintainability of this; it feels like we're doing back flips trying to avoid copy/pasting ~50 lines of code
- Are people using this feature? Do people know about this feature?
- What if one of us just made an
npmpackage that handled this for us; it could be anindex.jsfile and aindex.test.jsfile living innpmthat we could depend on. Could that help us reduce the complexity in Perseus?
Open to push-back and will approve after you double-check the bundle size alerts, but I'm feeling like this feature is drifting from the KISS principle.
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (1dddd30) and published it to npm. You Example: pnpm add @khanacademy/perseus@PR2322If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR2322 |
|
How does moving this logic to a package fix the ESLint issue? |
handeyeco
left a comment
There was a problem hiding this comment.
I like it, thanks Jeremy!
| "@khanacademy/perseus-core": "workspace:*" | ||
| "@khanacademy/perseus-core": "workspace:*", | ||
| "@khanacademy/perseus-utils": "workspace:*", | ||
| "perseus-build-settings": "workspace:*" |
There was a problem hiding this comment.
I noticed that sometimes this is a dev dep and here it's a regular dep. Is one preferred over the other?
There was a problem hiding this comment.
purseus-build-settings should be a dev dep! I'll review and fix where it's wrong.
There was a problem hiding this comment.
I also caught one of perseus-utils in the wrong spot and fixed that too!
| # @khanacademy/perseus-utils | ||
|
|
||
| This is a container package for utilities that are used by all, or at least | ||
| most Perseus packages. |
There was a problem hiding this comment.
Worth mentioning that it can't depend on other Perseus packages?
| "directory": "packages/perseus-utils" | ||
| }, | ||
| "bugs": { | ||
| "url": "https://github.com/Khan/perseus/issues" |
There was a problem hiding this comment.
Random thought: do we even have the issues tab enabled?
There was a problem hiding this comment.
We do! Not sure that's good or bad. I'll leave this here for now. :)
Previously, this shared logic was included in each package's source tree as a symlinked folder. (bug report) This PR changes things so that the shared code is in a regular npm package that all of the other packages use. |
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @khanacademy/kas@0.5.1 ### Patch Changes - [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled - Updated dependencies \[[`4bd882b43`](4bd882b)]: - @khanacademy/perseus-utils@0.0.2 ## @khanacademy/keypad-context@1.1.7 ### Patch Changes - [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled - Updated dependencies \[[`5b6e9df5b`](5b6e9df), [`ca06cb806`](ca06cb8), [`4bd882b43`](4bd882b)]: - @khanacademy/perseus-core@5.4.2 - @khanacademy/perseus-utils@0.0.2 ## @khanacademy/kmath@0.4.7 ### Patch Changes - [#2328](#2328) [`5b6e9df5b`](5b6e9df) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Fix import of internal items to use relative paths instead of the package name - [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled - Updated dependencies \[[`5b6e9df5b`](5b6e9df), [`ca06cb806`](ca06cb8), [`4bd882b43`](4bd882b)]: - @khanacademy/perseus-core@5.4.2 - @khanacademy/perseus-utils@0.0.2 ## @khanacademy/math-input@23.0.6 ### Patch Changes - [#2296](#2296) [`7b76274f0`](7b76274) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Fix expression widget styling issues. Close button focus outline is now visible, backspace button styling is now consistent with other buttons, and adjusted the popover padding. - [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled - Updated dependencies \[[`5b6e9df5b`](5b6e9df), [`ca06cb806`](ca06cb8), [`4bd882b43`](4bd882b)]: - @khanacademy/perseus-core@5.4.2 - @khanacademy/keypad-context@1.1.7 - @khanacademy/perseus-utils@0.0.2 ## @khanacademy/perseus@57.2.2 ### Patch Changes - [#2296](#2296) [`7b76274f0`](7b76274) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Fix expression widget styling issues. Close button focus outline is now visible, backspace button styling is now consistent with other buttons, and adjusted the popover padding. - [#2301](#2301) [`11a3b8b24`](11a3b8b) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Fix number line input outline to meet accessible contrast standards - [#2328](#2328) [`5b6e9df5b`](5b6e9df) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Fix import of internal items to use relative paths instead of the package name - [#2326](#2326) [`2e26c0872`](2e26c08) Thanks [@nishasy](https://github.com/nishasy)! - [SR] Remove instructions for static graphs, mark interactive elements as disabled - [#2324](#2324) [`7a60db8e8`](7a60db8) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Adding roles to the Image Widget to help improve A11Y. - [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled - Updated dependencies \[[`7b76274f0`](7b76274), [`5b6e9df5b`](5b6e9df), [`ca06cb806`](ca06cb8), [`4bd882b43`](4bd882b)]: - @khanacademy/math-input@23.0.6 - @khanacademy/kmath@0.4.7 - @khanacademy/perseus-core@5.4.2 - @khanacademy/perseus-score@2.3.7 - @khanacademy/kas@0.5.1 - @khanacademy/keypad-context@1.1.7 - @khanacademy/perseus-linter@1.3.7 - @khanacademy/perseus-utils@0.0.2 - @khanacademy/pure-markdown@0.4.1 - @khanacademy/simple-markdown@0.14.1 ## @khanacademy/perseus-core@5.4.2 ### Patch Changes - [#2328](#2328) [`5b6e9df5b`](5b6e9df) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Fix import of internal items to use relative paths instead of the package name - [#2321](#2321) [`ca06cb806`](ca06cb8) Thanks [@benchristel](https://github.com/benchristel)! - Allow `itemDataVersion` to be null when parsing Perseus assessment item JSON - [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled - Updated dependencies \[[`4bd882b43`](4bd882b)]: - @khanacademy/kas@0.5.1 - @khanacademy/perseus-utils@0.0.2 ## @khanacademy/perseus-editor@18.2.2 ### Patch Changes - [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled - Updated dependencies \[[`7b76274f0`](7b76274), [`11a3b8b24`](11a3b8b), [`5b6e9df5b`](5b6e9df), [`2e26c0872`](2e26c08), [`7a60db8e8`](7a60db8), [`ca06cb806`](ca06cb8), [`4bd882b43`](4bd882b)]: - @khanacademy/math-input@23.0.6 - @khanacademy/perseus@57.2.2 - @khanacademy/kmath@0.4.7 - @khanacademy/perseus-core@5.4.2 - @khanacademy/perseus-score@2.3.7 - @khanacademy/kas@0.5.1 - @khanacademy/keypad-context@1.1.7 - @khanacademy/perseus-linter@1.3.7 - @khanacademy/perseus-utils@0.0.2 - @khanacademy/pure-markdown@0.4.1 ## @khanacademy/perseus-linter@1.3.7 ### Patch Changes - [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled - Updated dependencies \[[`5b6e9df5b`](5b6e9df), [`ca06cb806`](ca06cb8), [`4bd882b43`](4bd882b)]: - @khanacademy/perseus-core@5.4.2 - @khanacademy/perseus-utils@0.0.2 ## @khanacademy/perseus-score@2.3.7 ### Patch Changes - [#2328](#2328) [`5b6e9df5b`](5b6e9df) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Fix import of internal items to use relative paths instead of the package name - [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled - Updated dependencies \[[`5b6e9df5b`](5b6e9df), [`ca06cb806`](ca06cb8), [`4bd882b43`](4bd882b)]: - @khanacademy/kmath@0.4.7 - @khanacademy/perseus-core@5.4.2 - @khanacademy/kas@0.5.1 - @khanacademy/perseus-utils@0.0.2 ## @khanacademy/perseus-utils@0.0.2 ### Patch Changes - [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled ## @khanacademy/pure-markdown@0.4.1 ### Patch Changes - [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled - Updated dependencies \[[`4bd882b43`](4bd882b)]: - @khanacademy/perseus-utils@0.0.2 - @khanacademy/simple-markdown@0.14.1 ## @khanacademy/simple-markdown@0.14.1 ### Patch Changes - [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled - Updated dependencies \[[`4bd882b43`](4bd882b)]: - @khanacademy/perseus-utils@0.0.2 ## perseus-build-settings@0.5.1 ### Patch Changes - [#2322](#2322) [`4bd882b43`](4bd882b) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Change how version injection code is shared/bundled ## @khanacademy/perseus-dev-ui@5.3.7 ### Patch Changes - Updated dependencies \[[`7b76274f0`](7b76274), [`5b6e9df5b`](5b6e9df), [`ca06cb806`](ca06cb8), [`4bd882b43`](4bd882b)]: - @khanacademy/math-input@23.0.6 - @khanacademy/kmath@0.4.7 - @khanacademy/perseus-core@5.4.2 - @khanacademy/kas@0.5.1 - @khanacademy/perseus-linter@1.3.7 - @khanacademy/pure-markdown@0.4.1 - @khanacademy/simple-markdown@0.14.1 Author: khan-actions-bot Reviewers: ivyolamit Required Reviewers: Approved By: ivyolamit Checks: ⏭️ 1 check has been skipped, ✅ 4 checks were successful Pull Request URL: #2330
Summary:
A while ago, we decided it would be useful to inject the built version of each package into the package itself. This getrs surfaced on the
__perseus_debug__object attached to thewindow(globalThis) object at runtime. This is handy!Originally we implemented it in
@khanacademy/perseus-coreas that was a small, no-deps package. Over time, we've started moving more and more functionality into perseus-core (especially as part of server-side scoring). This ended up causing a circular dependency.To solve this, we moved the injection code inot the
/utilsfolder and symlinking it into each package. That worked... until we needed to enable the Typescript parser for eslint (see: typescript-eslint/typescript-eslint#2987).So this PR changes things once more. I've re-introduced a package which we can use for small, focused utilities. I know utils can sometimes become dumping grounds, but we can also be disciplined. :)
Issue: --none--
Test plan:
Run
pnpm buildBuild size check is stable (in fact, the bundles shrink a bit because things aren't integrated/bundled until the consuming app bundles things now).