Skip to content

feat: serve any componentpack file #4894

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

Merged
merged 1 commit into from
May 28, 2025
Merged

Conversation

techfg
Copy link
Collaborator

@techfg techfg commented May 28, 2025

What does this PR do?

Allows any file within a component pack to be served.

Previously, serving files from within a componentpack was limited to the regex [a-zA-Z0-9\\-_]+\\.(?:json|js|xml|txt|css){1}(?:\\.map)?}" so in short, only files with a single . and were of the listed extensions could be served. This limitations seems to be somewhat historical but presents issues when componentpacks need to contain other files that need to be served.

There does not seem to be any reason for limiting the files served from a component pack (the component pack review process should vet and discover anything that it not appropriate) so in order to support all uses cases, any file inside of a componentpack can be served.

Additionally, the Content-Disposition header filename property will contain the actual filename of the file being served instead of the generic pack.js. The Content-Disposition isn't really required since files within the component pack aren't being "downloaded" via the browser but leaving the filename property in-place for now, just with the correct name of the file.

Testing

Tested locally and all tests pass. e2e & ci will cover rest.

@techfg techfg requested a review from humandad May 28, 2025 03:16
@zachelrath
Copy link
Contributor

There does not seem to be any reason for limiting the files served from a component pack (the component pack review process should vet and discover anything that it not appropriate) so in order to support all uses cases, any file inside of a componentpack can be served.

I am trying to remember why we limited this originally, and I think I agree in principle. I don't think that there's a way for the Component Pack "pack" process to include files that aren't relevant, but I'm not sure. It would be good to ensure that we're not allowing gigantic / malicious files to be served, since it's the app server that's serving the files --- at least in workspace mode, I don't remember if Component Pack files in bundles get served directly from S3?? Either way, I think this is ok.

@techfg
Copy link
Collaborator Author

techfg commented May 28, 2025

Thanks for the background and input @zachelrath!

The use case that led to this was being able to serve a web worker that a component I'm working on requires. Outside of hosting it on a CDN, there really isn't a good solution for this. Given that, by opening up the ability to serve any file from within a component pack, the worker can be served directly from the pack.

As for "packing" the extra files, you are correct, uesio pack won't include these. In short, after pack, a custom script needs to manually copy the files to the dist folder and then deploy will include everything in dist. The component pack "build" process is something Ben and I have been discussing regarding potential improvements on and having a "stock" ability to include "extra" files is something on the list of improvements in addition to things like importing images (e.g., svg) as mentioned in #4388. Unfortunately, esbuild is pretty limited in support of these types of things, especially when using the go api so we'll likely need to implement a different "build" process all together when we do get to that.

In meantime, agreed on the concerns around file size, etc. but, at least for now, this can be mitigated by the component pack review & approval process.

@techfg techfg merged commit dcbddb0 into main May 28, 2025
6 checks passed
@techfg techfg deleted the feat/serve-any-componentpack-file branch May 28, 2025 20:18
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.

3 participants