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

Make assets available in a standalone directory to serve hydrogen-view-sdk/assets/ #682

Closed
MadLittleMods opened this issue Feb 17, 2022 · 0 comments · Fixed by #693
Closed
Assignees
Labels

Comments

@MadLittleMods
Copy link
Contributor

MadLittleMods commented Feb 17, 2022

Is your feature request related to a problem? Please describe.

It's hard to serve the static assets like the SVG icons and fonts because they are spread out amongst the rest of the package files in the root package directory.

Describe the solution you'd like

  • Have all assets available to serve under hydrogen-view-sdk/assets/
  • Also add an empty index.js like hydrogen-view-sdk/assets/index.js so require.resolve('hydrogen-view-sdk/assets/') can resolve the path properly.
  • Then assets can simply be served like app.use(express.static(path.dirname(require.resolve('hydrogen-view-sdk/assets/'))));

In order to avoid the following deprecation notice, we probably want to use this instead:

  • Add "./assets/*": "./asset-build/assets/*" to the list of package.json exports
  • require.resolve('hydrogen-view-sdk/assets/index.js')
(node:46028) [DEP0148] DeprecationWarning: Use of deprecated folder mapping "./assets/" in the "exports" field module resolution of the package at C:\Users\MLM\Documents\GitHub\element\matrix-public-archive\node_modules\hydrogen-view-sdk\package.json.
Update this package.json to use a subpath pattern like "./assets/*".

Describe alternatives you've considered

Vite does have SSR support for running in Node.js but I didn't want to dive into all of that complexity yet as it's not necessary. Might switch over though.

Additional context

This makes hydrogen-view-sdk easier to work with if you're not using Vite although the SDK specifies that only Vite is supported though: "Currently, only vite is supported" -- doc/SDK.md.

hydrogen-view-sdk currently doesn't even support CommonJS yet because of a Vite bug. Re-introducing CommonJS support is tracked by #686

https://github.com/vector-im/hydrogen-web/blob/0ff1a01b429799e5ade9d21e0186ab0980b4add0/scripts/sdk/create-manifest.js#L5-L26

MadLittleMods added a commit that referenced this issue Feb 26, 2022
Fix #686
Fix #682

Instead of deleting the whole `target/` directory, leave it alone so the symlink
driving the `npm link`/`yarn link` stays in tact.

Leave Vite builds in their build directories (`/lib-build`/`/asset-build`)
so you can `vite build --watch` to build on local changes and still have a
consisent place to reference in the `package.json` `exports`. Previously,
everything relied on `build.sh` which does a bunch of moving and renaming
and made it hard to rebuild on changes.

Add back support for CommonJS (adding the `package.json` `exports`).

The last piece is making sure the `?url` imports (`import workerPath from 'hydrogen-view-sdk/main.js?url';`)
work still. It looks like this may have just been solved via
vitejs/vite#6725 -> vitejs/vite#7073
(literally 2 days ago) and we just need to wait for the next Vite release 🎉
@MadLittleMods MadLittleMods self-assigned this Feb 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant