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

Upgrade glob to 10.4.2 #78

Merged
merged 2 commits into from
Jul 2, 2024
Merged

Upgrade glob to 10.4.2 #78

merged 2 commits into from
Jul 2, 2024

Conversation

memark
Copy link
Contributor

@memark memark commented Jul 2, 2024

When typechecking (npx tsc -b) the project locally, an error is reported

node_modules/path-scurry/dist/cjs/index.d.ts:115:31 - error TS2720: Class 'PathBase' incorrectly implements class 'Dirent'. Did you mean to extend 'Dirent' and inherit its members as a subclass?
  Property 'path' is missing in type 'PathBase' but required in type 'Dirent'.

115 export declare abstract class PathBase implements Dirent {
                                  ~~~~~~~~

  node_modules/@types/node/fs.d.ts:247:9
    247         path: string;
                ~~~~
    'path' is declared here.

This is fixed in newer versions of path-scurry, which we pull in via glob.

Updating glob solves the issue.

@obi1kenobi
Copy link
Owner

Ah, you'll have to compile the typescript source into a compiled JS artifact in dist/. This is an unfortunate but standard workflow for GitHub Actions, since the action runners can only run JS, not TS.

Run npm run build and commit & push the changes it made, and this should pass CI.

Thanks for digging into this, I appreciate it! If you'd be open to making another PR, I'd also love a ## Contributing section in the README (or a separate CONTRIBUTING.md file) that describes the steps you had to take, since in retrospect they are not obvious.

@memark
Copy link
Contributor Author

memark commented Jul 2, 2024

Ah, got it. Makes sense.

Thanks for the explanation. I'll look into creating a small file for contributing. (I do think it should be separate, given that the README is what also shows up at the Github actions marketplace.)

@obi1kenobi
Copy link
Owner

I do think it should be separate, given that the README is what also shows up at the Github actions marketplace.

Good call! I forgot about the marketplace preview.

@obi1kenobi obi1kenobi enabled auto-merge (squash) July 2, 2024 15:45
@obi1kenobi obi1kenobi merged commit bb0df35 into obi1kenobi:main Jul 2, 2024
29 checks passed
@memark memark deleted the upgrade-glob branch July 2, 2024 15:47
@memark memark mentioned this pull request Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants