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

JavaScript files not loading in dev mode #141

Closed
rabser opened this issue Oct 17, 2022 · 9 comments
Closed

JavaScript files not loading in dev mode #141

rabser opened this issue Oct 17, 2022 · 9 comments

Comments

@rabser
Copy link

rabser commented Oct 17, 2022

I don't know how to submit an issue for the paid version (which we own, version 2022052300/1.12.1 ) of levelup-xp (local/xp):
The drops table is missing the "secret" header so showing the drop table in settings, the secret is not printed out, but the secret is now the right way of create a drop (not the name anymore).

So after adding in "classes/output/drop_table.php"
'secret' => get_string('dropsecret', 'local_xp'),
to the $headers array, I got the secrets for each drop visible to teachers.

The lang files should be updated accordingly by adding the dropsecret string definition and also the drop_helps should be changed (and maybe the plugin documentation on the web), as by now a drop seems to be defined as [xpdrop drop_secret] and not by [xpdrop drop_name].

HTH

@rabser
Copy link
Author

rabser commented Oct 17, 2022

Update: sorry, i didn't realize that the modal-drop-setup.js was not loading ... so probably take to the trash the change to the code, but be aware that all the plugin documentation seems to point out that a xpdrop is built with it's name: the secret is never used.

@rabser
Copy link
Author

rabser commented Oct 17, 2022

Another update: why there's no ...js.map files in amd/build at all ? This is blocking js from loading correctly AFAIK. I've managed to load the minified js from amd/src (which is the loaded one if the .map is missing) and noticed that they differs: src scripts lacks of the define() at top.
Anyway, modal-drop-setup.js in amd/src is missing define() which prevents moodle from loading it, And the same is into the block_xp/amd/src/role-button.js ...

@FMCorz
Copy link
Owner

FMCorz commented Oct 18, 2022

Hi @rabser,

Thank you for sharing these details.

The drops have always required their secret in their snippet, however we do not expect users to write the snippet themselves that is why the secret is not displayed as is. The other reason is to keep the secret hidden. To obtain the code snippet you can simply click the name of the drop in the drop table.

We apologise if there are some lang strings that refer to the drop name instead of the secret, would you please share their string IDs as we cannot identify which ones those are.

The .js.map files are not required for JS to load. However if you are using a version of Moodle prior to 3.8, JavaScript assets should be cached or they won't load. This is a known issue that has recently been reported in #140. In more recent versions, Moodle has started requiring that JavaScript is always built and is no longer loaded from the src/ folder.

@rabser
Copy link
Author

rabser commented Oct 19, 2022

To be honest, it's not written "[xpdrop name]" but "[xpdrop abcdef]" which can lead the reader to misunderstand. I suggest to change to "[xpdrop secretecode]" in the lang of local_xp. Probably we thinked - erroneusly - that the shortcodes had to refer to the drop name (it's what's you choose when you create the drop) and the documentation does'nt mention the secret code so we had a mind shortcircuit ...
For the js.map missing, we found that we had the cachejs set to false (this is a 3.11.10 for testing/developer instance). Enabling the js caching all went fine. This oddities could be overcome if the js files (either modal-drop-setup.min.js and role-button.js) into the amd/src will contain the define() as the other js' have, letting us to run in developer mode.

HTH

@FMCorz
Copy link
Owner

FMCorz commented Oct 20, 2022

Thank you for your comment. You're right, there may be confusion with how the shortcode is constructed, we should improve the documentation and help in that regard.

Regarding JavaScript, that is rather odd. The define calls are no longer expected in src files, since 3.8 JS files are to be built even during development, but if you had cachejs set to false then the built files should have loaded. Missing .js.map files should not prevent them from loading.

We'll investigate that, thank you.

Reference: https://docs.moodle.org/dev/Javascript_Modules#Development_mode_.28Moodle_v3.8_and_above.29

@rabser
Copy link
Author

rabser commented Oct 20, 2022

Thanks for your prompt reply, if you look at the logic in lib/requirejs.lib, you'll find that going in development mode, if the .js.map is not present, the js file is indeed read from amd/src. If the src is missing the define() a debugging error is fired (last else).
If I simply copy the define() function into the corresponding src, all the errors disappear.

@FMCorz
Copy link
Owner

FMCorz commented Oct 20, 2022

Oh wow, thank you so much for pointing that out! I frankly had no idea about this behaviour. This is extremely helpful, we'll make sure that the map files are included to prevent this from happening, as indeed, module files cannot be included directly.

@rabser
Copy link
Author

rabser commented Oct 20, 2022

I'm pretty raw in my behaviour :-), so I overwrote the src one with the corresponding build file and all works fine either with or without the developer mode enabled... the only difference between them is the define() line, that's why I was suggesting the littler change to your code. But obviously, take your choice freely.

@FMCorz FMCorz changed the title Found a bug in levelup-xp+ JavaScript files not loading in dev mode Dec 12, 2022
@FMCorz FMCorz closed this as completed in 5c2ab4d Dec 12, 2022
@FMCorz
Copy link
Owner

FMCorz commented Dec 12, 2022

Thank you for your report, we've included the .map files, that should issues going forward, at least on recent Moodle versions.

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