GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||
GeezerVison is showing github.com — fix: Compile TS to JS during packaging by camdecoster · Pull Request #8000 · plotly/plotly.js Conversation
Contributor
DescriptionCompile TS down to JS during packaging (before publishing) to fix issue with Node resolution. Closes #7995. Also fixes TS issues discovered during investigation:
Changes
Testing
npm pack --pack-destination /tmp mkdir -p /tmp/plotly-verify && cd /tmp/plotly-verify && npm init -y && npm i /tmp/plotly.js-4.0.0.tgz node -e "console.log(require.resolve('plotly.js/src/lib/mod'))"
Notes
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
KoolADE85
reviewed
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Left a couple comments on the testing, but the solution itself is working well on my end.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Comment on lines
+11
to
+12
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Claude took me down this path as well while I was testing this out. To me, it feels a bit "one step removed" from a real-world validation. Could we instead write a more idiomatic
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was all an attempt to run without
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
|
| const { createRequire } = require('node:module'); | ||
| const path = require('node:path'); |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This was all an attempt to run without npm install, but it's too complicated. I'll remove it in favor of GHA.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
You're right. I liked having the test locally, but it does fit better in GHA. I'll remove this file.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
💃
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
[BUG]: v4.0.0 publishes uncompiled .ts sources that are required extensionless — package fails to load under Node's CJS resolver
There was an error while loading. Please reload this page.