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 ignore option to ignore files from build #68

Merged

Conversation

Mikescops
Copy link

Fixes #67

This is an attempt to solve the issue #67 by adding a new option in pkg called ignore, it let's define regex paths that will avoid files matching them to be included in the final bundle.

Feel free to add comments and suggestions on how to improve the logic, I have included a simple test as a demo.

lib/walker.ts Outdated Show resolved Hide resolved
@robertsLando robertsLando changed the title Ignore patterns of included files feat: ignore patterns of included files May 31, 2024
Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

Please also update README.md to document the new setting

lib/index.ts Outdated Show resolved Hide resolved
@Mikescops
Copy link
Author

Please also update README.md to document the new setting

Done and I made changes requested.

Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

LGTM.

@robertsLando robertsLando changed the title feat: ignore patterns of included files feat: add ignore option to ignore files while building Jun 3, 2024
@robertsLando robertsLando changed the title feat: add ignore option to ignore files while building feat: add ignore option to ignore files from build Jun 3, 2024
@robertsLando
Copy link
Member

robertsLando commented Jun 3, 2024

@Mikescops Tests are failing: https://github.com/yao-pkg/pkg/actions/runs/9320724918/job/25736985706?pr=68#step:8:25

I think the problem is that pnpm has dropped support for nodej 16 and we don't specify a specific pnpm version so it installs latest and so that error. Could you fix please?

@Mikescops
Copy link
Author

@robertsLando done

Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

Tests are still failing and this time I think it's related to your changes:

https://github.com/yao-pkg/pkg/actions/runs/9351769172/job/25738734963?pr=68#step:8:41

lib/index.ts Outdated Show resolved Hide resolved
@Mikescops
Copy link
Author

There is a last error unrelated to this PR, still concerning pnpm, on windows that fails. If you have ideas.

https://github.com/yao-pkg/pkg/actions/runs/9353352423/job/25743740134

lib/options.ts Outdated Show resolved Hide resolved
@robertsLando
Copy link
Member

@Mikescops EINVAL suggests there is a wrong argument passed to the command, I'm not a windows guy so I cannot help much here but I maybe for some reason npx on widows doesn't correctly parse the @8?

@Mikescops
Copy link
Author

Mikescops commented Jun 4, 2024

So (18.x, windows-latest) is still breaking on this thing, it's hitting my nerves.

But the (16.x, windows-latest) is failing on my new tests, probably due to paths with \

@Mikescops
Copy link
Author

Found this: nexe/nexe#1091

will try

@robertsLando robertsLando enabled auto-merge (squash) June 4, 2024 11:40
@robertsLando robertsLando merged commit 54ae1ee into yao-pkg:main Jun 4, 2024
6 checks passed
@Mikescops Mikescops deleted the feature/ignore-patterns-of-included-files branch June 4, 2024 12:50
@Mikescops
Copy link
Author

We got there 😅 thanks

@robertsLando
Copy link
Member

Thanks to you 🙏🏼

@Mikescops
Copy link
Author

@robertsLando can you tag a new release with the change? thanks 🙏

@robertsLando
Copy link
Member

Sure, 5.12.0 it's coming now

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.

pkg packing uneeded build files for native modules
2 participants