Skip to content
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

feat: add scss and react support in import order eslint rule #241

Merged
merged 5 commits into from Mar 14, 2022

Conversation

vovakulikov
Copy link
Contributor

@vovakulikov vovakulikov commented Mar 13, 2022

Background

Prior to this PR, our current eslint config allowed us to have the following imports order

import { voronoi } from '@visx/voronoi'
import { noop } from 'lodash'
import React, { ReactElement, useMemo, useRef, useState } from 'react'

import styleHello from './world-styles'
import styles from './local.module.scss'
import hello from './world'

As you can see the syles imports and react imports are mixed with other group packages. Which makes the import section a bit confusing.

Since in react component React is the main import it should be the first import in the file.
Since styles import has the biggest specific to the component it should be the last import in the file.

import React, { ReactElement, useMemo, useRef, useState } from 'react'

import { voronoi } from '@visx/voronoi'
import { noop } from 'lodash'

import styleHello from './world-styles'
import hello from './world'

import styles from './local.module.scss'

@vovakulikov vovakulikov self-assigned this Mar 13, 2022
@vovakulikov vovakulikov changed the title Add scss and react support in import order eslint rule fix: add scss and react support in import order eslint rule Mar 13, 2022
Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

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

Awesome! Thank you for taking the time to update it.

The flow of fixing the errors in the codebase that I used:

  1. Merge eslint-config PR.
  2. New version of the config will be published to NPM.
  3. Upgrade the eslint-config version in the sourcegraph repo.
  4. Run eslint --fix in the sourcegraph repo.
  5. Create PR with sourcegraph repo changes.
  6. 🚀

Not sure what caused errors on the CI pipeline, and it seems unrelated to PR changes. Let's create an issue to investigate if re-running the pipeline doesn't help.

@vovakulikov vovakulikov changed the title fix: add scss and react support in import order eslint rule feat: add scss and react support in import order eslint rule Mar 14, 2022
@vovakulikov vovakulikov merged commit 9b383bc into master Mar 14, 2022
@vovakulikov vovakulikov deleted the vk/update-import-order branch March 14, 2022 14:09
@sourcegraph-buildkite
Copy link
Collaborator

🎉 This PR is included in version 0.27.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants