feat: add `compile` option to enable rerun of transpilation for react templates #1177

Gmin2 · 2024-04-04T06:53:39Z

Description

The PR introduces a new option compile in the Generator class contructor which is by default false ,by default the React template files are not transpiled. When setting the compile options to true the transpilation process takes place.

I have also added/updated test for it. Below are the result

Related issue(s)
Related to #521

which added this functionality

lib/generator.js

lib/renderer/react.js

derberg · 2024-04-09T10:20:37Z

lib/generator.js

@@ -391,7 +393,7 @@ class Generator {
   */
  async configureTemplate() {
    if (isReactTemplate(this.templateConfig)) {
-      await configureReact(this.templateDir, this.templateContentDir, TRANSPILED_TEMPLATE_LOCATION);
+      await configureReact(this.templateDir, this.templateContentDir, TRANSPILED_TEMPLATE_LOCATION, this.compile);


I'm wondering if it makes sense to modify configureReact or maybe better just do conditional here, like

if (isReactTemplate(this.templateConfig) && this.compile) { await configureReact(this.templateDir, this.templateContentDir, TRANSPILED_TEMPLATE_LOCATION);

I have added conditional transpilation

lib/renderer/react.js

lib/generator.js

sonarcloud · 2024-04-10T05:36:22Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

derberg

ok, we need to add a proper new integration test here, without proper test, feature development will be based on just our intuition

there are some integration tests already, that use https://github.com/asyncapi/html-template template

we just need another test, also using https://github.com/asyncapi/html-template but fixed to use version 2.3.0 where __transpiled folder was added to package.

there are 2 test scenarios:

with compile flag false
you run html template generation with debug flag
console.log message should contain something about Babel
with compile flag true
you run html template generation with debug flag
console.log message should not contain something about Babel

Why? We know html-template as some big files and babel warns about it in logs, when transpilation runs. Now, we also know that we do not need to run transpilation as __transpiled dir is there. This is why we can, with such integrations test, confirm if transpilation is triggered only if compile is true

makes sense?

Gmin2 · 2024-04-11T08:32:03Z

ok, we need to add a proper new integration test here, without proper test, feature development will be based on just our intuition

there are some integration tests already, that use https://github.com/asyncapi/html-template template

we just need another test, also using https://github.com/asyncapi/html-template but fixed to use version 2.3.0 where __transpiled folder was added to package.

there are 2 test scenarios:

with compile flag false
you run html template generation with debug flag
console.log message should contain something about Babel

with compile flag true
you run html template generation with debug flag
console.log message should not contain something about Babel

Why? We know html-template as some big files and babel warns about it in logs, when transpilation runs. Now, we also know that we do not need to run transpilation as __transpiled dir is there. This is why we can, with such integrations test, confirm if transpilation is triggered only if compile is true

makes sense?

looks alright

derberg · 2024-04-16T10:54:27Z

@utnim2 ok then, ping me when ready for another review

derberg · 2024-04-22T08:18:22Z

@utnim2 do you continue with this one?

Gmin2 · 2024-04-22T08:20:19Z

@utnim2 do you continue with this one?

currently i am having my semester exam, will continue working on it after my exams are over
cc @derberg

Gmin2 · 2024-05-18T17:30:01Z

Hey @derberg, i guess compile flag is not working as expected

the integration test fails here and logs does not contain something about Babel, and generating react-template test also failing currently investgating it

reference of test taken from this

derberg · 2024-05-20T11:13:14Z

/update

derberg

since last time, there were changes in integration tests and we should not rely on html-template anymore, you need to use https://github.com/asyncapi/generator/tree/master/test/test-templates/react-template

and yeah, in that template there are no babel logs we can rely on.

so maybe the best will be to simply extend configureReact with a log message (only if debug flag) that basically drops log like "Transpilation of files DIR into OUTPUT_DIR started"

then probably enough is to add just unit test in generator.test.js, check but probably integration test is not needed

one more problem, compile as you put in title, should rerun transpilation. So if someone makes some modification in template and needs to refresh __transpiled dir, they need to pass this flag.

so compilation always runs by default if __transpiled dir is not present

we need to have this also documented then in template-development.md (maybe other docs too):

that when people publish templates based on react, they should include __transpiled dir in the package
that during development they need to pass the flag to run compile explicitly to refresh __transpiled dir when needed

derberg · 2024-05-20T11:17:11Z

lib/renderer/react.js

@@ -13,6 +13,7 @@ const reactExport = module.exports;
 * @param {string} templateLocation located for thetemplate
 * @param {string} templateContentDir where the template content are located
 * @param {string} transpiledTemplateLocation folder for the transpiled code
+ * @param {Boolean} compile Whether to compile the template files or used the cached transpiled version provided by the template in the '__transpiled' folder


did you do some changes in below code? cause now this jsdoc is not reflecting reality 🤔

derberg · 2024-05-20T11:21:54Z

test/integration.test.js

+      debug: true,
+    });
+
+    const logSpy = jest.spyOn(console, 'log').mockImplementation(() => {});


to be consistent in project, for spying on console, please follow the same approach we have in generator.test.js

log.debug = jest.fn();
expect(log.debug).toHaveBeenCalledWith("redacted for brevity));

derberg · 2024-05-20T11:23:48Z

test/integration.test.js

+    const outputDir = generateFolderName();
+    const generator = new Generator('@asyncapi/html-template@2.3.0', outputDir, {
+      forceWrite: true,
+      compile: false,


it is false by default, so in test you should also not pass it

sonarcloud · 2024-05-22T07:57:50Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Gmin2 · 2024-05-22T11:11:21Z

so maybe the best will be to simply extend configureReact with a log message (only if debug flag) that basically drops log like "Transpilation of files DIR into OUTPUT_DIR started"

then probably enough is to add just unit test in generator.test.js, check but probably integration test is not needed

i am confused here, so do we need to add integration test or do we just need to add test in generator.test.js

cc @derberg

sonarcloud · 2024-06-20T14:33:43Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

derberg · 2024-06-24T09:08:49Z

i am confused here, so do we need to add integration test or do we just need to add test in generator.test.js

just try with suggestion to add log and test for it - if it will work, no integration test is needed

Gmin2 · 2024-06-29T07:40:37Z

@derberg any idea why workflow is failing? (idk why test seems to fail)

Gmin2 · 2024-07-01T14:28:05Z

Hey @derberg when i run locally, with compile true then it works perfectly

but dont know why test is failing

derberg

left few comments, some things we talked about during meeting - you did not take into account

also, you as contributor and maintainer need to proactively check and see that PR tests are failing. Me as maintainer, if I see tests are failing, I do not even start review as my assumption is PR is not ready for review (I reviewed this time only because we talked about PR and I saw you did some commits after)

apps/generator/lib/renderer/react.js

apps/generator/test/generator.test.js

apps/generator/test/integration.test.js

derberg · 2024-07-29T12:13:30Z

@Gmin2 your tests are still failing

changeset-bot · 2024-07-31T12:28:21Z

⚠️ No Changeset found

Latest commit: b3df084

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

…emplate

sonarcloud · 2024-07-31T13:48:43Z

Quality Gate failed

Failed conditions
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

Gmin2 requested review from derberg, magicmatatjahu, jonaslagoni and asyncapi-bot-eve as code owners April 4, 2024 06:53

This was referenced Apr 4, 2024

React templates make generation process longer #521

Open

[BUG] Request a review could ignore bot asyncapi/.github#286

Closed

derberg changed the title ~~feat: added a compile option to enable skipping default transpilation in react~~ feat: add compile option to enable rerun of transpilation for react templates Apr 9, 2024

derberg reviewed Apr 9, 2024

View reviewed changes

lmgyuan reviewed Apr 9, 2024

View reviewed changes

lib/renderer/react.js Outdated Show resolved Hide resolved

lmgyuan reviewed Apr 9, 2024

View reviewed changes

lib/renderer/react.js Outdated Show resolved Hide resolved

lmgyuan reviewed Apr 9, 2024

View reviewed changes

lib/generator.js Outdated Show resolved Hide resolved

derberg reviewed Apr 11, 2024

View reviewed changes

derberg reviewed May 20, 2024

View reviewed changes

derberg assigned Gmin2 Jun 19, 2024

derberg reviewed Jul 15, 2024

View reviewed changes

Gmin2 added 19 commits July 31, 2024 18:20

added a compile option to enable skipping default transpilation

bbe0a7b

updated jsdoc and implemented conditional transpilation in configureT…

565eefb

…emplate

added some changes

cb0ca24

added integration testing for compile flag

ef94410

added a compile option to enable skipping default transpilation

492814e

updated jsdoc and implemented conditional transpilation in configureT…

77cc9d1

…emplate

rebased:added some changes

d5c986c

remove integration test

237b14a

fixed linting problem

830d195

added console logs in configureReact and added test

13aea56

added the integration test and cleanup

792fc3c

remove the unit test

1dbf3db

remove temp.md

b4037f4

added integration test to be created during the testing

cee60c2

cleanup

d4589aa

updated the test

0407e86

make the reviewed changes

d3f1ba9

fixed linting prob

ebbf27e

removed the __transpiled folder content for cleanup of test

39169e0

Gmin2 force-pushed the compile branch from 26743d5 to 39169e0 Compare July 31, 2024 12:57

Gmin2 and others added 2 commits July 31, 2024 19:09

renamed the corrected name

0245b65

Merge branch 'master' into compile

b3df084

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add `compile` option to enable rerun of transpilation for react templates #1177

feat: add `compile` option to enable rerun of transpilation for react templates #1177

Gmin2 commented Apr 4, 2024 •

edited

Loading

derberg Apr 9, 2024

Gmin2 Apr 9, 2024

sonarcloud bot commented Apr 10, 2024

derberg left a comment

Gmin2 commented Apr 11, 2024

derberg commented Apr 16, 2024

derberg commented Apr 22, 2024

Gmin2 commented Apr 22, 2024

Gmin2 commented May 18, 2024

derberg commented May 20, 2024

derberg left a comment

derberg May 20, 2024

derberg May 20, 2024

derberg May 20, 2024

sonarcloud bot commented May 22, 2024

Gmin2 commented May 22, 2024

sonarcloud bot commented Jun 20, 2024

derberg commented Jun 24, 2024

Gmin2 commented Jun 29, 2024

Gmin2 commented Jul 1, 2024

derberg left a comment

derberg commented Jul 29, 2024

changeset-bot bot commented Jul 31, 2024 •

edited

Loading

sonarcloud bot commented Jul 31, 2024

feat: add compile option to enable rerun of transpilation for react templates #1177

Are you sure you want to change the base?

feat: add compile option to enable rerun of transpilation for react templates #1177

Conversation

Gmin2 commented Apr 4, 2024 • edited Loading

derberg Apr 9, 2024

Choose a reason for hiding this comment

Gmin2 Apr 9, 2024

Choose a reason for hiding this comment

sonarcloud bot commented Apr 10, 2024

Quality Gate passed

derberg left a comment

Choose a reason for hiding this comment

Gmin2 commented Apr 11, 2024

derberg commented Apr 16, 2024

derberg commented Apr 22, 2024

Gmin2 commented Apr 22, 2024

Gmin2 commented May 18, 2024

derberg commented May 20, 2024

derberg left a comment

Choose a reason for hiding this comment

derberg May 20, 2024

Choose a reason for hiding this comment

derberg May 20, 2024

Choose a reason for hiding this comment

derberg May 20, 2024

Choose a reason for hiding this comment

sonarcloud bot commented May 22, 2024

Quality Gate passed

Gmin2 commented May 22, 2024

sonarcloud bot commented Jun 20, 2024

Quality Gate passed

derberg commented Jun 24, 2024

Gmin2 commented Jun 29, 2024

Gmin2 commented Jul 1, 2024

derberg left a comment

Choose a reason for hiding this comment

derberg commented Jul 29, 2024

changeset-bot bot commented Jul 31, 2024 • edited Loading

⚠️ No Changeset found

sonarcloud bot commented Jul 31, 2024

Quality Gate failed

feat: add `compile` option to enable rerun of transpilation for react templates #1177

feat: add `compile` option to enable rerun of transpilation for react templates #1177

Gmin2 commented Apr 4, 2024 •

edited

Loading

changeset-bot bot commented Jul 31, 2024 •

edited

Loading