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 true ,by default the React template files are transpiled. When setting the compile options to false the transpilation process does not takes place.

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

Related issue(s)
Closes #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

🦋 Changeset detected

Latest commit: 834b3cc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@asyncapi/generator	Minor

Not sure what this means? Click here to learn what changesets are.

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

derberg · 2024-08-11T18:03:14Z

@Gmin2 any progress?

derberg · 2024-08-20T21:22:21Z

apps/generator/lib/logMessages.js

@@ -59,5 +63,7 @@ module.exports = {
  templateSuccessfullyInstalled,
  relativeSourceFileNotGenerated,
  conditionalFilesMatched,
+  compileEnabled,
+  compileEnabled,


doubled export

derberg · 2024-08-20T21:29:50Z

apps/generator/cli.js

@@ -84,6 +84,7 @@ program
  })
  .option('-d, --disable-hook [hooks...]', 'disable a specific hook type or hooks from given hook type', disableHooksParser)
  .option('--debug', 'enable more specific errors in the console')
+  .option('--compile', 'compile the template files before rendering')


after all works remember to remove compile flag from here as we do not enable new flags in local cli

test: update tests

…mpile

sonarcloud · 2024-08-26T12:15:01Z

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-08-26T12:15:32Z

@Gmin2 all good, I just improved the changeset a bit to make it clear what this release is all about. Please have a look for future education

derberg · 2024-08-26T12:41:32Z

/rtm

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

Closed

[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 and others added 3 commits August 14, 2024 01:52

Merge remote-tracking branch 'upstream/master' into compile

5bd97e1

Merge branch 'master' into compile

da7cc75

tests: update tests

2f6e74f

derberg reviewed Aug 20, 2024

View reviewed changes

Gmin2 and others added 8 commits August 21, 2024 11:28

Merge pull request #1 from derberg/compile

04522a5

test: update tests

Merge branch 'compile' of https://github.com/utnim2/generator into co…

8e5b6d0

…mpile

Merge branch 'master' into compile

cc0bf09

Update package-lock.json

7627f56

Merge remote-tracking branch 'upstream/master' into compile

f37b920

removed compile from cli.js and fixed double import

f817b42

Merge remote-tracking branch 'origin/compile' into compile

9dbb7b7

added the changeset for the release

d98a38d

Gmin2 requested a review from Florence-Njeri as a code owner August 21, 2024 09:38

derberg and others added 7 commits August 21, 2024 18:26

Update cli.js

5e9efbe

Update package-lock.json

8082c8e

Update test-project.test.js

7f2af96

Update test-registry.test.js

6e5cc87

making the compile option by default true

cfda26f

Merge branch 'compile' of https://github.com/utnim2/generator into co…

7123a04

…mpile

a small test change

7b3c1a9

Gmin2 mentioned this pull request Aug 26, 2024

[FEATURE]: Make compile option default to false in future major release #1259

Open

2 tasks

Update compile_option.md

834b3cc

derberg approved these changes Aug 26, 2024

View reviewed changes

asyncapi-bot added the ready-to-merge label Aug 26, 2024

asyncapi-bot merged commit 46114d8 into asyncapi:master Aug 26, 2024
15 checks passed

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 by derberg

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

derberg commented Aug 11, 2024

derberg Aug 20, 2024

derberg Aug 20, 2024

sonarcloud bot commented Aug 26, 2024

derberg commented Aug 26, 2024

derberg commented Aug 26, 2024

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

Conversation

Gmin2 commented Apr 4, 2024 • edited by derberg 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

🦋 Changeset detected

derberg commented Aug 11, 2024

derberg Aug 20, 2024

Choose a reason for hiding this comment

derberg Aug 20, 2024

Choose a reason for hiding this comment

sonarcloud bot commented Aug 26, 2024

Quality Gate passed

derberg commented Aug 26, 2024

derberg commented Aug 26, 2024

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 by derberg

Loading

changeset-bot bot commented Jul 31, 2024 •

edited

Loading