Skip to content

Conversation

@ivov
Copy link
Member

@ivov ivov commented Apr 21, 2023

@github-actions
Copy link
Contributor

Great PR! Please pay attention to the following items before merging:

Files matching packages/**:

  • If fixing bug, added test to cover scenario.
  • If addressing forum or Github issue, added link to description.

Files matching packages/**/*.ts:

  • Added unit tests to cover new or updated functionality.

Files matching **/*.vue:

  • Used composition API for all new components.
  • Added component or unit tests to cover functionality.

Files matching packages/design-system/**/*.vue:

  • Used design system tokens (colors, spacings...) where possible.
  • Updated Storybook with new component or updated functionality.

Make sure to check off this list before asking for review.

@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system labels Apr 21, 2023
@codecov
Copy link

codecov bot commented Apr 21, 2023

Codecov Report

Patch coverage: 92.41% and no project coverage change.

Comparison is base (feb2ba0) 18.59% compared to head (b380e61) 18.59%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6060   +/-   ##
=======================================
  Coverage   18.59%   18.59%           
=======================================
  Files        2585     2585           
  Lines      116668   116678   +10     
  Branches    18181    18181           
=======================================
+ Hits        21690    21697    +7     
- Misses      94340    94343    +3     
  Partials      638      638           
Impacted Files Coverage Δ
...n-system/src/components/N8nFormInput/validators.ts 0.00% <0.00%> (ø)
...ents/N8nNodeCreatorNode/NodeCreatorNode.stories.ts 0.00% <0.00%> (ø)
...ents/N8nRecycleScroller/RecycleScroller.stories.ts 0.00% <0.00%> (ø)
...m/src/components/N8nUsersList/UsersList.stories.ts 0.00% <0.00%> (ø)
packages/design-system/src/types/datatable.ts 0.00% <0.00%> (ø)
.../src/components/SettingsLogStreaming/Helpers.ee.ts 0.00% <0.00%> (ø)
...components/SettingsLogStreaming/descriptions.ee.ts 0.00% <0.00%> (ø)
packages/editor-ui/src/mixins/restApi.ts 0.00% <0.00%> (ø)
...tor-ui/src/plugins/codemirror/completions/types.ts 0.00% <0.00%> (ø)
packages/editor-ui/src/Interface.ts 100.00% <100.00%> (ø)
... and 83 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@github-actions
Copy link
Contributor

✅ All Cypress E2E specs passed

@ivov ivov merged commit 57aab63 into master Apr 24, 2023
@ivov ivov deleted the n8n-6054-consistent-type-imports-in-fe branch April 24, 2023 10:18
Copy link
Contributor

@OlegIvaniv OlegIvaniv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one! I found a few cases where we could adjust/remove imports. It seems like the linting is not triggered for imports which are not used

import { IDataObject, INodeTypeDescription, ITaskData, NodeHelpers } from 'n8n-workflow';
import type { INodeTypeDescription, ITaskData } from 'n8n-workflow';
import { IDataObject, NodeHelpers } from 'n8n-workflow';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IDataObject is type, but it's not used in the file

XYPosition,
} from '@/Interface';
import type { IExecutionsSummary, INodeUi, XYPosition } from '@/Interface';
import { INodeUpdatePropertiesInformation } from '@/Interface';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

INodeUpdatePropertiesInformation is type, but it's not used in the file

} from 'n8n-workflow';
import {
import type { INodeTypeDescription, INodeActionTypeDescription } from 'n8n-workflow';
import { INodeTypeNameVersion } from 'n8n-workflow';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

INodeTypeNameVersion is type but not used in this file

INodePropertyCollection,
NodeParameterValueType,
} from 'n8n-workflow';
import { NodeHelpers, NodeParameterValue } from 'n8n-workflow';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NodeParameterValue is type but not used in the file

NodeParameterValueType,
} from 'n8n-workflow';
import { INodeUi, IUpdateInformation, TargetItem } from '@/Interface';
import { IRunData, isResourceLocatorValue } from 'n8n-workflow';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IRunData is type but not used in the file

import type { Schema, Optional, Primitives } from '@/Interface';
import { isObj } from '@/utils/typeGuards';
import { generatePath } from '@/utils/mappingUtils';
import { DateTime } from 'luxon';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DateTime is a type but not used in this file

import { showMessage } from '@/mixins/showMessage';
import { ICredentialsResponse, ICredentialTypeMap, IUser } from '@/Interface';
import type { ICredentialsResponse, ICredentialTypeMap } from '@/Interface';
import { IUser } from '@/Interface';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a type but not used in the file

} from 'n8n-workflow';
import type { IExecutionsCurrentSummaryExtended, IRestApiContext } from '@/Interface';
import type { ExecutionFilters, ExecutionOptions, IDataObject } from 'n8n-workflow';
import { ExecutionStatus, WorkflowExecuteMode } from 'n8n-workflow';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are types but not used in the file

@ivov
Copy link
Member Author

ivov commented Apr 24, 2023

Nice one! I found a few cases where we could adjust/remove imports. It seems like the linting is not triggered for imports which are not used

Thanks for reviewing! Addressed in the two linked PRs above.

MiloradFilipovic added a commit that referenced this pull request Apr 24, 2023
* master:
  refactor: Async functions don't need to explicitly return promises (no-changelog) (#6041)
  SSO/SAML : add Base URL to redirects in acsHandler (#5923)
  refactor: Integrate `consistent-type-imports` in FE packages (no-changelog) (#6060)
  fix(core): Skip license activation when instance was already activated (#6064)
  refactor(core): Setup decorator based RBAC (no-changelog) (#5787)

# Conflicts:
#	packages/editor-ui/src/mixins/restApi.ts
MiloradFilipovic added a commit that referenced this pull request Apr 24, 2023
* master:
  refactor(editor): Delete leftover restApi mixin file (no-changelog) (#6074)
  docs: Remove version notice from overhauled nodes (no-changelog) (#6071)
  refactor(editor): Combine type imports in `editor-ui` (no-changelog) (#6072)
  refactor: Async functions don't need to explicitly return promises (no-changelog) (#6041)
  SSO/SAML : add Base URL to redirects in acsHandler (#5923)
  refactor: Integrate `consistent-type-imports` in FE packages (no-changelog) (#6060)
  fix(core): Skip license activation when instance was already activated (#6064)
  refactor(core): Setup decorator based RBAC (no-changelog) (#5787)
  test: Add run linking tests (#6061)

# Conflicts:
#	packages/editor-ui/src/components/ParameterInputWrapper.vue
sunilrr pushed a commit to fl-g6/qp-n8n that referenced this pull request Apr 24, 2023
…gelog) (n8n-io#6060)

* 👕 Move `consistent-type-imports` to top level

* 👕 Apply lintfixes

* 👕 Apply more lintfixes

* 👕 More lintfixes

* 👕 More lintfixes
@janober
Copy link
Member

janober commented May 2, 2023

Got released with [email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team Released ui Enhancement in /editor-ui or /design-system

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants