Skip to content

CLOUDP-298233: use JS instead of cjs #427

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.

Sign up for GitHub

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

Merged
wtrocki merged 5 commits into from
Feb 17, 2025
Merged

CLOUDP-298233: use JS instead of cjs #427

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d235968
CLOUDP-298233: use JS instead of cjs
wtrocki Feb 14, 2025
c4fad28
Update tools/.tool-versions
wtrocki Feb 14, 2025
cca7996
fix: format
wtrocki Feb 14, 2025
794fd8a
ensure node in package.json
wtrocki Feb 17, 2025
41320e6
Merge branch 'main' into CLOUDP-298233-postman
wtrocki Feb 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,9 @@
"globals": "^15.14.0",
"jest": "^29.7.0",
"prettier": "3.5.0"
},
"engineStrict": false,
"engines": {
"node": ">=20.0.0"
}
}
1 change: 1 addition & 0 deletions tools/.tool-versions
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
golang 1.23.0
nodejs 20.9.0
Copy link
Collaborator

@lovisaberggren lovisaberggren Feb 14, 2025

Choose a reason for hiding this comment

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

Looked at the package json reference, we can specify there if we want: https://docs.npmjs.com/cli/v11/configuring-npm/package-json#engines

Copy link
Member Author

@wtrocki wtrocki Feb 17, 2025

Choose a reason for hiding this comment

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

Yes. That is very common practice. Adding it.

Copy link
Contributor

@gssbzn gssbzn Feb 17, 2025

Choose a reason for hiding this comment

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

adding it to the package.json just produces warnings

2 changes: 1 addition & 1 deletion tools/postman/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ transform_collection:

.PHONY: transform_collection_js
transform_collection_js:
node ./scripts/transform-postman.cjs
node ./scripts/transform-postman.js

.PHONY: transform_collection_test
transform_collection_test:
Expand Down
6 changes: 3 additions & 3 deletions tools/postman/scripts/transform-postman.cjs → tools/postman/scripts/transform-postman.js
View file
Open in desktop
Copy link
Contributor

@gssbzn gssbzn Feb 14, 2025

Choose a reason for hiding this comment

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

node uses commonjs by default should this be .mjs

Copy link
Collaborator

@lovisaberggren lovisaberggren Feb 14, 2025

Choose a reason for hiding this comment

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

As I understand it esm is the standard nowadays

Copy link
Member Author

@wtrocki wtrocki Feb 14, 2025

Choose a reason for hiding this comment

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

We have all files as js (which should be fine) using (mjs) import format.
This PR makes single change to align to that format.

If we want to change extension or import format let me know. Can do it across repo comprehensively.

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const fs = require('fs');
const path = require('path');
const _ = require('lodash');
import fs from 'fs';
import path from 'path';
import _ from 'lodash';

/**
# Prepare collection for Postman API
Expand Down
Loading