-
Notifications
You must be signed in to change notification settings - Fork 3.3k
fix: remove 'new' badge from Create from Component, fix parsing #31457
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
Conversation
@@ -33,6 +33,7 @@ export class CodegenActions { | |||
let result = parseReactComponent(src, { | |||
resolver: findAllWithLink(exportResolver, reactDocgenResolvers), | |||
babelOptions: { | |||
configFile: false, |
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.
This option disables the use of a config file for Babel. this is what we want since we're calling it directly, we don't want to pick up any config file that might be in the project
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.
is it possible to add a test for this?
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.
@AtofStryker I added a test in CodegenActions.spec.ts
just to make sure that we pass this option to Babel. I went down the road of trying to do a system test for this instead, but couldn't get it to work
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.
its probably hard to do a system test since its code generation for a component? Have you tried a cy-in-cy test? That would give us the most coverage here
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.
@astone123 any luck with the cy-in-cy test?
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.
no I haven't looked at it yet, will try it out this week
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.
@AtofStryker I finally added a test for this - let me know what you think
cypress
|
Project |
cypress
|
Branch Review |
create-from-component-fix
|
Run status |
|
Run duration | 16m 55s |
Commit |
|
Committer | astone123 |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
2
|
|
133
|
|
0
|
|
5552
|
View all changes introduced in this branch ↗︎ |
@@ -33,6 +33,7 @@ export class CodegenActions { | |||
let result = parseReactComponent(src, { | |||
resolver: findAllWithLink(exportResolver, reactDocgenResolvers), | |||
babelOptions: { | |||
configFile: false, |
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.
is it possible to add a test for this?
acdcf8e
to
d362468
Compare
@@ -4,7 +4,6 @@ | |||
:header="t('createSpec.component.importFromComponent.header')" | |||
:description="t('createSpec.component.importFromComponent.description')" | |||
:icon="DocumentCodeIcon" | |||
:badge-text="t('versions.new')" |
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.
this isn't new anymore
…from Component, fix parsing (#31457)
…om Component, fix parsing (#31457)
…from Component, fix parsing (#31457)
…om Component, fix parsing (#31457)
…from Component, fix parsing (#31457)
…om Component, fix parsing (#31457)
* chore: updating v8 snapshot cache * index on release/15.0.0: 47bda54 fix: remove 'new' badge from Create from Component, fix parsing (#31457) * index on release/15.0.0: 3b88383 internal: (studio) support creating new test in root suite (#32024) --------- Co-authored-by: cypress-bot[bot] <+cypress-bot[bot]@users.noreply.github.com>
* chore: updating v8 snapshot cache * index on release/15.0.0: 47bda54 fix: remove 'new' badge from Create from Component, fix parsing (#31457) * index on release/15.0.0: 3b88383 internal: (studio) support creating new test in root suite (#32024) --------- Co-authored-by: cypress-bot[bot] <+cypress-bot[bot]@users.noreply.github.com>
* chore: updating v8 snapshot cache * index on release/15.0.0: 47bda54 fix: remove 'new' badge from Create from Component, fix parsing (#31457) * index on release/15.0.0: 3b88383 internal: (studio) support creating new test in root suite (#32024) --------- Co-authored-by: cypress-bot[bot] <+cypress-bot[bot]@users.noreply.github.com>
Additional details
This PR removes the "New" badge for the Create from Component feature - we released it over two years ago. It also fixes an issue where we would fail to parse component files for certain projects that had babel configurations in their project root. You can see this issue with
cypress-services
dashboard tests - if you try to use Create from Component with any of these files, it will error on all of them because of Babel plugin issues from the project's Babel config. We don't want to use the project's configuration file.Steps to test
On develop
cypress-services/frontend/packages/dashboard
in CTPR Tasks
cypress-documentation
?type definitions
?