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

Create version file and delete loading package.json version #798

Merged

Conversation

camcam-lemon
Copy link
Contributor

@camcam-lemon camcam-lemon commented Nov 30, 2024

📑 Summary

  • Create version file of src/version.js.
  • Changed version reference from package.json to version.js.
  • Added step to update src/version.js at github actions.

Resolves #796

📏 Design Decisions

Using JSON Modules proved difficult, so we decided to create a version constant file

📋 Tasks

Make sure you

  • 📖 have read the contribution guidelines
  • 💻 have added unit/e2e tests (if appropriate)
  • 🔖 targeted master branch

Comment on lines 45 to 50

- name: Update version file
run: |
echo "export const version = '${{env.RELEASE_VERSION}}'" > src/version.js
git add src/version.js

Copy link
Member

Choose a reason for hiding this comment

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

Hmmmm, I don't think this will work, since the git commit is done in the npm version command, so it's already too late to call git add.

How about something like the following:

diff --git a/.github/workflows/release-publish.yml b/.github/workflows/release-publish.yml
index 85d3720..f07a2d1 100644
--- a/.github/workflows/release-publish.yml
+++ b/.github/workflows/release-publish.yml
@@ -43,11 +43,6 @@ jobs:
       - name: Prepare release
         run: npm version --no-git-tag-version --allow-same-version ${{env.RELEASE_VERSION}}

-      - name: Update version file
-        run: |
-          echo "export const version = '${{env.RELEASE_VERSION}}'" > src/version.js
-          git add src/version.js
-
       - name: Convert repository name to lower case
         run: echo "GITHUB_REPOSITORY_LOWER_CASE=$(echo $GITHUB_REPOSITORY | tr '[:upper:]' '[:lower:]')" >> $GITHUB_ENV

diff --git a/package.json b/package.json
index d879480..3de1d76 100644
--- a/package.json
+++ b/package.json
@@ -25,6 +25,7 @@
     "prepare": "tsc && vite build",
     "prepack": "tsc && vite build",
     "test": "NODE_OPTIONS=\"$NODE_OPTIONS --experimental-vm-modules\" npx jest",
+    "version": "node scripts/version.js",
     "lint": "standard",
     "lint-fix": "standard --fix"
   },
diff --git a/scripts/version.js b/scripts/version.js
new file mode 100755
index 0000000..0787efa
--- /dev/null
+++ b/scripts/version.js
@@ -0,0 +1,24 @@
+#!/usr/bin/env node
+
+/**
+ * This script reads the version from package.json and writes it to src/version.js.
+ * It also stages the changes to src/version.js in git.
+ *
+ * Should be used as an [`npm version` script](https://docs.npmjs.com/cli/v8/using-npm/scripts#npm-version)!
+ */
+import { execFile } from 'node:child_process'
+import { readFile, writeFile } from 'node:fs/promises'
+import { promisify } from 'node:util'
+
+async function main () {
+  const packageJson = JSON.parse(await readFile('package.json', 'utf8'))
+  const version = packageJson.version
+
+  await writeFile('src/version.js', `export const version = '${version}'\n`)
+  await promisify(execFile)('git', ['add', 'src/version.js'])
+}
+
+main().catch((error) => {
+  console.error(error)
+  process.exit(1)
+})

If you read https://docs.npmjs.com/cli/v10/commands/npm-version#description, it sounds like when calling npm version, the version script in our package.json file is called:

  • After version in package.json has been updated (so we can safely do const packageJson = JSON.parse(await readFile('package.json', 'utf8')))
  • Before the git commit/git tag is made.

It's also easier for you to test! You can try running npm version prepatch and see if it works!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I guess that’s true now that you bring it up.
Apparently, I had the wrong idea about how the npm version command behaves.
Your approach is simpler and makes testing easier perfect!!
I fix my code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed! 82236e2

Changed it because it is easier and more testable than doing it from github actions
Copy link
Member

@aloisklink aloisklink left a comment

Choose a reason for hiding this comment

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

I tested it locally with npm verison prepatch and it worked!

Thanks for the amazing and well-documented PR! If you're ever in the Kantō region of Japan, I owe you a coffee/beer as thanks 😄

@aloisklink aloisklink added fix and removed feature labels Dec 2, 2024
@aloisklink aloisklink merged commit d12939c into mermaid-js:master Dec 2, 2024
5 checks passed
@aloisklink
Copy link
Member

And it's been released in v11.4.1!

Feel free to look at the code at https://www.npmjs.com/package/@mermaid-js/mermaid-cli/v/11.4.1?activeTab=code

This PR made it just in time for the release 😜

I hope this solves your issue, and if it doesn't, feel free to make another issue/PR!

@camcam-lemon
Copy link
Contributor Author

Thank you so much for quickly merging and publishing!
I'm super happy because it removed one build step from the library I'm working on!!!
If I'm ever in the area, let's definitely grab a coffee!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request:Update package.json to Use import' with {type: 'json'} Instead of require
2 participants