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

fix(assets): Explicit mode allows all non-JS file extensions #85

Merged
merged 5 commits into from
Aug 14, 2019

Conversation

calebdwilliams
Copy link
Contributor

@FredKSchott Here is the commit discussed in #76 that allows non-JS assets to be moved to web_modules/ in explicit mode.

Copy link
Owner

@FredKSchott FredKSchott left a comment

Choose a reason for hiding this comment

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

This is looking great, really excited to see this land. Left some comments inline

.gitignore Outdated Show resolved Hide resolved
package-lock.json Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@calebdwilliams
Copy link
Contributor Author

@FredKSchott I've added a commit with the requested changes. Hopefully I got everything. If not, let me know and I'll take care of that this evening.

Copy link
Owner

@FredKSchott FredKSchott left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, just checked it out to run it and noticed a few small behavioral things

src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@FredKSchott
Copy link
Owner

This is great! LGTM, thanks for making the change. Looking forward to using this myself on www.pika.dev.

The only other part of this that I'd like to get in soon is glob matching ("webDependencies": ["some-package/assets/*.css"]) to address #49. Let me know if you have any interest in tackling that, otherwise I'll take a look later this week/weekend.

Once that's in we can let it all bake a bit, and then decide if we want to go deeper into the "webDependencies" config (with automatic package.json "style" entrypoint detection) or have a seperate "assets" config for styles, images, etc.

@FredKSchott FredKSchott merged commit d115064 into FredKSchott:master Aug 14, 2019
@pika-ci
Copy link

pika-ci bot commented Aug 14, 2019

🚀 This PR has been merged! Once a new version is released, it will become available on npm. Until then, you can load and install it directly from the Pika CDN:

npm install https://cdn.pika.dev/@pika/web/master

@calebdwilliams
Copy link
Contributor Author

@FredKSchott FWIW, I would like to also take a look at the glob settings if that's cool. I really like this package and it meets a lot of my needs so I'd love to contribute, though it might be next week before I can get to it.

1 similar comment
@calebdwilliams
Copy link
Contributor Author

@FredKSchott FWIW, I would like to also take a look at the glob settings if that's cool. I really like this package and it meets a lot of my needs so I'd love to contribute, though it might be next week before I can get to it.

@FredKSchott
Copy link
Owner

FredKSchott commented Aug 15, 2019

@calebdwilliams Great! Really appreciate the contributions. Just ping me whenever you want to discuss or when you have something ready to take a look at

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