generator icon indicating copy to clipboard operation
generator copied to clipboard

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

Open Gmin2 opened this issue 1 year ago • 14 comments

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

image

Related issue(s) Related to #521

image

which added this functionality

Gmin2 avatar Apr 04 '24 06:04 Gmin2

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

Gmin2 avatar Apr 11 '24 08:04 Gmin2

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

derberg avatar Apr 16 '24 10:04 derberg

@utnim2 do you continue with this one?

derberg avatar Apr 22 '24 08:04 derberg

@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 avatar Apr 22 '24 08:04 Gmin2

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

image

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

Gmin2 avatar May 18 '24 17:05 Gmin2

/update

derberg avatar May 20 '24 11:05 derberg

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

Gmin2 avatar May 22 '24 11:05 Gmin2

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

derberg avatar Jun 24 '24 09:06 derberg

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

Gmin2 avatar Jun 29 '24 07:06 Gmin2

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

but dont know why test is failing image

Gmin2 avatar Jul 01 '24 14:07 Gmin2

@Gmin2 your tests are still failing

derberg avatar Jul 29 '24 12:07 derberg

🦋 Changeset detected

Latest commit: 834b3ccdccbe413a9badce4ddb0490785320e165

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

changeset-bot[bot] avatar Jul 31 '24 12:07 changeset-bot[bot]

@Gmin2 any progress?

derberg avatar Aug 11 '24 18:08 derberg

@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 avatar Aug 26 '24 12:08 derberg

/rtm

derberg avatar Aug 26 '24 12:08 derberg