Skip to content

[HOLD for payment 2023-11-09] [$1000] Setup sorted import aliases #20531

Closed

Description

@roryabraham

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Coming from this slack thread, this issue is just a DX / code beautification thing, and thus should be treated an non-urgent / low priority.

Problem

When writing code for the new Expensify app, our goal is to provide developers with a positive coding experience, enhancing their joy and productivity.
The most commonly used development tool is Visual Studio Code (VSCode). However, VSCode often struggles to suggest imports for custom Expensify components. As a result, developers have to manually type the import path. Since the components they are working on are often nested in various folders, they have to guess the number of directories they need to go up to import the desired component ("Do I need to go up three directories like ../../../ or just two?"). This unnecessary cognitive load distracts developers from their actual tasks. Furthermore, the cluttered import paths are difficult to read and comprehend (see the first picture).

Solution

Take a look at the second picture and let your heart decide which version appears cleaner to you. The proposed solution is called import alias.
We can configure the bundlers to use "shortcuts" to directories when resolving imports. Additionally, we can instruct ESLint to prioritize the use of these shorter import aliases, ensuring automatic detection during development.
Next time you need to use an Expensify component, such as , you can simply import it swiftly using the following syntax:
import Avatar from "@components/Avatar"
(Future note: once we integrate typescript VSCode should be able to resolve all import paths, and thus it will suggest us to import the component from @components/Avatar )

Before After
image image

Furthermore, let's setup an (auto-fixable by Prettier) sort-imports lint rule, to make it look like this:

image

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b1a5b3c310973924
  • Upwork Job ID: 1697273497918955520
  • Last Price Increase: 2023-09-01
  • Automatic offers:
    • situchan | Reviewer | 26446972
    • b4s36t4 | Contributor | 26446976

Activity

  1. melvin-bot commented on Jun 9, 2023

    @melvin-bot
  2. added and removed
    NewFeatureSomething to build that is a new item.
    on Jun 9, 2023
  3. roryabraham commented on Jun 9, 2023

    @roryabraham
    ContributorAuthor

    This is more of a developer-experience optimization, not really a new feature.

  4. hannojg commented on Jun 9, 2023

    @hannojg
    Contributor

    👋

  5. 87 remaining items

  6. melvin-bot commented on Oct 31, 2023

    @melvin-bot

    The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.93-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

    If no regressions arise, payment will be issued on 2023-11-07. 🎊

    After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

    • External issue reporter
    • Contributor that fixed the issue
    • Contributor+ that helped on the issue and/or PR

    For reference, here are some details about the assignees on this issue:

  7. bfitzexpensify commented on Nov 7, 2023

    @bfitzexpensify
    Contributor

    Deployed to prod on 11/2, assuming no regressions, payment will be due on 11/9

  8. changed the title [-][HOLD for payment 2023-11-07] [$1000] Setup sorted import aliases[/-] [+][HOLD for payment 2023-11-09] [$1000] Setup sorted import aliases[/+] on Nov 7, 2023
  9. bfitzexpensify commented on Nov 9, 2023

    @bfitzexpensify
    Contributor

    Payments complete. Given this was a developer QOL thing, I figure we likely don't need the BZ checklist here - is that correct @situchan?

  10. situchan commented on Nov 9, 2023

    @situchan
    Contributor

    @roryabraham should penalty still be applied here given the efforts made? This issue was specific than others. So not bug, not urgent and had to wait merge until no conflict and folks are online

  11. roryabraham commented on Nov 11, 2023

    @roryabraham
    ContributorAuthor

    I don't think that there was a regression here, so I don't think there should be any penalty. Even if there was a regression, I would be in support of waiving any penalty in light of the logistical challenges of doing a PR that touches so many files.

  12. situchan commented on Nov 12, 2023

    @situchan
    Contributor

    There was not regression but I think @bfitzexpensify applied because of not being merged in 9 days

  13. bfitzexpensify commented on Nov 13, 2023

    @bfitzexpensify
    Contributor

    Yeah, the Upwork job automatically updates after 9 days.

    The reasoning to not apply a penalty is reasonable to me. I've just sent bonus amounts of $500 to you both @situchan and @b4s36t4

    I think we can close this one out

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

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionDailyKSv2ExternalAdded to denote the issue can be worked on by a contributorTask

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions