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

Documented the case of a scoped package #17

Merged
merged 7 commits into from
Feb 10, 2023
Merged

Conversation

ijlee2
Copy link
Owner

@ijlee2 ijlee2 commented Feb 10, 2023

Description

Thanks to @phndiaye's pull request, I realized that the codemod has two bugs:

References

  • https://docs.npmjs.com/cli/v7/using-npm/workspaces#defining-workspaces

    Workspaces are usually defined via the workspaces property of the package.json file, e.g:

    {
      "name": "my-workspaces-powered-project",
      "workspaces": [
        "workspace-a"
      ]
    }

    Given the above package.json example living at a current working directory . that contains a folder named workspace-a that itself contains a package.json inside it, defining a Node.js package, e.g:

    .
    +-- package.json
    `-- workspace-a
       `-- package.json
  • https://classic.yarnpkg.com/lang/en/docs/workspaces/#toc-tips-tricks

    The workspaces field is an array containing the paths to each workspace. Since it might be tedious to keep track of each of them, this field also accepts glob patterns! For example, Babel reference all of their packages through a single packages/* directive.

  • https://pnpm.io/pnpm-workspace_yaml

    pnpm-workspace.yaml defines the root of the workspace and enables you to include / exclude directories from the workspace. By default, all packages of all subdirectories are included.

    For example:

    packages:
      # all packages in direct subdirs of packages/
      - 'packages/*'
      # all packages in subdirs of components/
      - 'components/**'
      # exclude packages that are inside test directories
      - '!**/test/**'

@ijlee2 ijlee2 added the bug Something isn't working label Feb 10, 2023
@ijlee2 ijlee2 force-pushed the bugfix-scoped-package branch from 2ccb45e to 7d98af2 Compare February 10, 2023 04:03
@@ -12,21 +12,22 @@ function normalizeRelativePath(relativePath) {
}

function updateFile(oldFile, { filePath, projectName, projectRoot }) {
const regex = new RegExp(`\\b${projectName}/(.*/)*(.*)\\b`, 'g');
const regex = new RegExp(`(?:'|")(${projectName}/(.*/)*(.*))(?:'|")`, 'g');
Copy link
Owner Author

@ijlee2 ijlee2 Feb 10, 2023

Choose a reason for hiding this comment

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

The word boundary \b in the beginning led to incorrect matches when the package name is scoped (because projectName starts with a non-word character @).

By being more specific about the boundaries (i.e. the file path has to be surrounded by single or double quotation marks), we can also avoid false positives, as evidenced in 0fee1ed.

@ijlee2 ijlee2 marked this pull request as ready for review February 10, 2023 04:24
@ijlee2 ijlee2 merged commit 746c269 into main Feb 10, 2023
@ijlee2 ijlee2 deleted the bugfix-scoped-package branch February 10, 2023 04:25
@phndiaye
Copy link

Thanks for this, @ijlee2 ! It will help a lot for the next addons I'm going to convert :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants