-
Notifications
You must be signed in to change notification settings - Fork 64
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
fix: compile nmp library as 'commonjs' module #1666
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: vitaliy-guliy <[email protected]>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: vitaliy-guliy The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
apply_sed 's/\"description\": \".*\"/"description": "Typescript types for devfile api"/g' $WORK_DIR/typescript-models/package.json | ||
apply_sed 's/\"repository\": \".*\"/"repository": "devfile\/api"/g' $WORK_DIR/typescript-models/package.json | ||
apply_sed 's/\"license\": \".*\"/"license": "Apache-2.0"/g' $WORK_DIR/typescript-models/package.json | ||
apply_sed 's/\"@types\/bluebird\": \".*\"/"@types\/bluebird": "3.5.21"/g' $WORK_DIR/typescript-models/package.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know if it's okay to remove
apply_sed 's/\"@types\/bluebird\": \".*\"/"@types\/bluebird": "3.5.21"/g' $WORK_DIR/typescript-models/package.json
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could have deleted it by accident.
I will double check it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recalled why did I remove it at all, there was no dependency on bluebird
in package.json
.
To check, go to https://www.npmjs.com/package/@devfile/api?activeTab=code and open existing package.json
Description of Changes
Compile generated
@devfile/api
node package ascommonjs
module instead ofnode:module
.It was around two years ago when the
@devfile/api
package was published ascommonjs
module https://www.npmjs.com/package/@devfile/api/v/2.2.1-alpha-1667236163?activeTab=codeSince that time all the node libraries generated by openapi-generator are being compiled as
node:module
.( go to https://www.npmjs.com/package/@devfile/api?activeTab=code, open
package.json
and look attype
property )Both Che Dashboard and Che Code projects are compiled as
commonjs
.There is a well known restriction, that
node:modue
cannot be included intocommonjs
module usingimport
,require
should be used instead.Che Dashboard and Che Code projects both use
@devfile/api
. When we package dashboard and che-code with webpack, we do not have any issues ( btw, this is a reason why we didn't have the problem for a long time). But when we try to compile the project without using webpack and then run it in a development mode (browser downloads a thousand of separate files), we face a problem with loading some files from@devfile/api
.The original issue highlighting a problem eclipse-che/che#23163
A workaround after which I decided to open this PR che-incubator/che-code#472
Tests Performed
Explain what tests you personally ran to ensure the changes are functioning as expected.
How To Test
Instructions for the reviewer on how to test your changes.
Notes To Reviewer
Any notes you would like to include for the reviewer.