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

Add support for module script tags to support Vite #10492

Open
NateWr opened this issue Oct 2, 2024 · 2 comments
Open

Add support for module script tags to support Vite #10492

NateWr opened this issue Oct 2, 2024 · 2 comments
Assignees

Comments

@NateWr
Copy link
Contributor

NateWr commented Oct 2, 2024

Describe the bug
I want to use Vite, a modern asset bundler, when developing a custom OJS theme. However, it is not possible to load the JavaScript assets using the TemplateManager::addJavaScript() function, because it does not support module script tags.

<script type="module" src="..."></script>

I can work around this in my own theme. However, I am preparing a Composer package to make the setup easier for the community. It would be best for this use case if I could integrate directly with the {load_scripts} template tag.

To Reproduce
I am aware that this is not a bug and I of all people campaigned to keep requests out of the issues. 😆 But I have a PR incoming.

What application are you using?
I'd like this to go back to OJS 3.3 if possible, for a client.

NateWr added a commit to NateWr/pkp-lib that referenced this issue Oct 2, 2024
@NateWr
Copy link
Contributor Author

NateWr commented Oct 2, 2024

The following PRs add this feature by extending the $args param of TemplateManager::addJavaScript(). To load a module script, you can use the following code:

$templateMgr->addJavaScript('example', 'path/to/example.js', ['type' => 'module']);

If you approve this change, I can prepare PRs for OMP/OPS as well as stable-3_4_0 and main.

PRs:
#10493 (stable-3_3_0)

Tests:
pkp/ojs#4456 (stable-3_3_0)

@jardakotesovec
Copy link
Contributor

@NateWr Hi Nate, I don't see reason not to include this PR.

Just to learn more, for ui-library we end up building 'iife' format, because with script type=module its not easy to enforce order with respect to other JS dependencies we load.

So just curious whether that possibly work for you in your scenario?

@jardakotesovec jardakotesovec self-assigned this Oct 7, 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

No branches or pull requests

2 participants