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

user function seems not to work on mobile #223

Closed
ebullient opened this issue May 16, 2021 · 21 comments
Closed

user function seems not to work on mobile #223

ebullient opened this issue May 16, 2021 · 21 comments
Assignees
Labels

Comments

@ebullient
Copy link

@ebullient ebullient commented May 16, 2021

Plugin informations (please complete the following information):

  • OS: iOS 14.4.2
  • Templater version: 1.6.0
  • Obsidian version: 0.0.19
  • Templater settings: {"command_timeout":5,"template_folder":"[assets]/templates","templates_pairs":[["",""]],"trigger_on_file_creation":true,"enable_system_commands":false,"shell_path":"","script_folder":"[assets]/scripts"}

Describe the bug
This will take some explaining. Let's start with what works everywhere. I have a daily template that looks like this:

<%* var titledate = tp.file.title;
const today = moment(titledate).isoWeekday();
    var nextworkday = moment(titledate).add(1, 'd');
    var thisweek = moment(titledate).day(1);
    var tomorrow = 'tomorrow';
    if (today === 1 ) {
        thisweek = moment(titledate);
    } else if (today > 4) {
        nextworkday = moment(titledate).startOf('isoWeek').add(1, 'week').day("monday");
        tomorrow = 'Monday';
    }
    tR +=  '# My Day\n'
        + moment(titledate).format("dddd, MMMM DD, YYYY") 
        + ' .... ['+tomorrow+'](' + nextworkday.format("YYYY-MM-DD")+')  \n'
        + 'Week of ['+thisweek.format("MMMM DD")+'](../weekly/Week-'+thisweek.format("YYYY-MM-DD")+')  \n';
%>

If I try to break this into a user function, it works on desktop, but not on mobile. Specifically, I tried replacing the template content with this:

<%* var titledate = tp.file.title;
const today = moment(titledate).isoWeekday();
tR += tp.user.daily(today, titledate); 
%>

(I use those variables elsewhere, which also seems not to work on mobile with the user function in the mix... works fine on desktop).

My user function (in the scripts folder configured above) is in a file called daily.js, and it looks like this:

function daily(today, titledate) {
    var nextworkday = moment(titledate).add(1, 'd');
    var thisweek = moment(titledate).day(1);
    var tomorrow = 'tomorrow';
    if (today === 1 ) {
        thisweek = moment(titledate);
    } else if (today > 4) {
        nextworkday = moment(titledate).startOf('isoWeek').add(1, 'week').day("monday");
        tomorrow = 'Monday';
    }
    return '# My Day\n'
        + moment(titledate).format("dddd, MMMM DD, YYYY") 
        + ' .... ['+tomorrow+'](' + nextworkday.format("YYYY-MM-DD")+')  \n'
        + 'Week of ['+thisweek.format("MMMM DD")+'](../weekly/Week-'+thisweek.format("YYYY-MM-DD")+')  \n';
}
module.exports = daily;

Expected behavior
would love to be able to simplify my templates and use functions, but it needs to work on mobile, too.

enhancement request: would be lovely to have a debug flag to turn on for mobile that would write a log.. hacking the plugin to spit out log files on mobile isn't my favorite (i worry about making the problem worse). a debug log that captured the information you need could make filing useful bug reports easier in general?)

@SilentVoid13
Copy link
Owner

@SilentVoid13 SilentVoid13 commented May 22, 2021

Hey @ebullient, you are correct user script functions are not working on mobile atm, but that's because I have to use the obsidian API FileSystemAdapter feature that isn't available on mobile.
Whenever the obsidian team adds the mobile equivalent to this feature, I'll add support for mobile.
Leaving this open until it's not implemented.

@ankushg
Copy link
Contributor

@ankushg ankushg commented May 28, 2021

@SilentVoid13 Is this.app.vault.adapter.getBasePath() the only method you need from FileSystemAdapter? Just want to make sure I understand what the current limitation is 😄

@SilentVoid13
Copy link
Owner

@SilentVoid13 SilentVoid13 commented Jun 17, 2021

Hey @ankushg, yes that's literally the only thing that I need but that's only available on desktop. I basically need the full path to the js file to import it. I couldn't find a solution to this problem, but if you found something I'd be curious to know.

@ruqqq
Copy link

@ruqqq ruqqq commented Jun 20, 2021

@SilentVoid13

I did these:

  1. Hardcoding the base path (I'm on android so it starts with /storage/emulated/0/Documents/Obsidian)
  2. Disable this code path
    if (Object.keys(window.require.cache).contains(file_path)) {
    (window.require is undefined on mobile)

Now I am getting the error: Failed to load user script ${file_path}. No exports detected.

Is import directive even supported on mobile/android?

@lishid
Copy link

@lishid lishid commented Jul 8, 2021

Hey @ankushg, yes that's literally the only thing that I need but that's only available on desktop. I basically need the full path to the js file to import it. I couldn't find a solution to this problem, but if you found something I'd be curious to know.

getBasePath does not exist on mobile because there is no basepath. On Mobile, we get wildly different paths depending on the platform - on Android we'll sometimes get content:// paths, sometimes get file:/// paths, and sometimes get /storage/paths. On iOS we get some weird http://localhost:port/ paths, or sometimes a file:/// path. The platform does some additional encoding of URIs depending on whether it's URI based or path based.

You'll want to use the getResourcePath if you want to load it as a URL in the browser; this will get you an URI that is handled by the platform that can load in the WebView. Otherwise, you should rely on vault.read(file) to get the contents, and run the contents of the file.

@chaseadamsio
Copy link
Contributor

@chaseadamsio chaseadamsio commented Jul 14, 2021

@SilentVoid13 I opened a PR to update the docs (#282) as a temporary measure to let folks know user functions aren't currently available on mobile.

@shabegom
Copy link
Collaborator

@shabegom shabegom commented Aug 20, 2021

@SilentVoid13 what if we tried something like:

const user_function_raw = this.app.vault.read(file)

const user_function = user_function_raw.replace(/module.exports(.+)\n/, "")

const fn = window["user_function"];
if(typeof fn === 'function') {
    this.user_script_functions.set(`${file.basename}`, fn);

Or if we don't have access to the window object use the Function constructor:

const fn = Function(user_function)

I'm not near a computer to try this out, but will test next week and open a PR if it works... one problem might be reading the JS file... not sure app.vault.read will do that.

@shabegom
Copy link
Collaborator

@shabegom shabegom commented Aug 24, 2021

Update: I tested it, and believe my approach above will work! Will submit the PR tomorrow for review.

@shabegom
Copy link
Collaborator

@shabegom shabegom commented Aug 24, 2021

PR #340 submitted

@beingskyler
Copy link

@beingskyler beingskyler commented Oct 1, 2021

Did this ever get resolved? Will gladly fund development if needed.

@ebullient
Copy link
Author

@ebullient ebullient commented Oct 1, 2021

As another option: I switched over to using the CustomJS plugin with both Templater and Dataview. It is a slightly different style (uses classes), but does the job. Given ubiquity, I would almost prefer Templater adopted that approach: If you want to use your own script, use CustomJS, and call them as you would anything else.. it reduces the scope of what Templater has to support (while still achieving the goal for users).

@Dyldog
Copy link

@Dyldog Dyldog commented Nov 11, 2021

@ebullient Are you showing any prompts from your CustomJS scripts? If so, how?

@lishid
Copy link

@lishid lishid commented Nov 11, 2021

@SilentVoid13 I took a look at how you're loading the script - I see that you're using the absolute path because you're using NodeJS's import function, which is why it'll never work on mobile.

The way I recommend you to change this for mobile support, is to load the file into a string using vault/adapter.read (relative to vault root), and then call eval.

The way we load plugins, for example, is to do this:

let js = await adapter.read(file_path);

let require = window.require;
let exports: any = {};
let module = {exports};

window.eval('(function anonymous(require,module,exports){' + js + '\n})');
fn(require, module, exports);

let result = exports['default'] || module.exports;

@ebullient
Copy link
Author

@ebullient ebullient commented Nov 11, 2021

@ebullient Are you showing any prompts from your CustomJS scripts? If so, how?

I use Templater if/when I need prompts, and CustomJS usually for computation/functions or custom filters, etc.

@shabegom
Copy link
Collaborator

@shabegom shabegom commented Nov 11, 2021

The way I recommend you to change this for mobile support, is to load the file into a string using vault/adapter.read (relative to vault root), and then call eval.

This is basically the approach I took in PR #340. I just don't think SV has had time to review/merge the change.

CustomJS is a decent workaround for now, it uses a similar approach (vault.read) so works on mobile.

@shabegom
Copy link
Collaborator

@shabegom shabegom commented Jan 13, 2022

Update: going to revisit my PR and merge if it still is working properly. Stay tuned!

@ebullient
Copy link
Author

@ebullient ebullient commented Jan 13, 2022

worth considering if CustomJS should just be the way. Scope creep is a thing. ;) (And CustomJS works with dataview, too.. )

@ebullient
Copy link
Author

@ebullient ebullient commented Jan 13, 2022

@ebullient Are you showing any prompts from your CustomJS scripts? If so, how?

Yes.. but I pass tp in, and use the existing tp mechanism to do it. Its a very nice mix and match.

@shabegom
Copy link
Collaborator

@shabegom shabegom commented Jan 24, 2022

Friends! SV has released 1.10.0 which fixes support for user scripts on mobile. Closing this issue! 🎉

@shabegom shabegom closed this Jan 24, 2022
@AB1908
Copy link

@AB1908 AB1908 commented Jan 29, 2022

@ebullient have you had a chance to test this out?

@ebullient
Copy link
Author

@ebullient ebullient commented Jan 30, 2022

No.. I've moved everything over to CustomJS in the meanwhile ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
10 participants