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 domtext work with static pages #1

Closed
tobibeer opened this issue Nov 23, 2015 · 5 comments
Closed

make domtext work with static pages #1

tobibeer opened this issue Nov 23, 2015 · 5 comments

Comments

@tobibeer
Copy link
Owner

@Jermolene, I am trying to build a static documentation for this little plugin that ships with a js macro.

(Where / How ) Can I adjust the domtext macro so that — upon static site generation — it knows how to handle document.getElementById()? Can this be done at all?

Are there some general instructions on how to make macros / widgets "static site proof"?

@Jermolene
Copy link

Hi @tobibeer as we've discussed it elsewhere, I guess you know that the macro doesn't comply with the rules for a macro because its output isn't purely a function of its parameters.

Anyhow, macros shouldn't reference the global document object (one reason being that it's not available under Node.js). Instead, use the document property of the widget object ie this.document.

It's not a foolproof check for static site generation, but you can check whether the document is TW's fakedom implementation, which would normally be sufficient:

if(this.document.isTiddlyWikiFakeDom) {
...
}

@tobibeer
Copy link
Owner Author

Thanks, @Jermolene, I imagined it involved fakedom, although I expected it would ship with its own getElementById() and then saw that it doesn't.

its output isn't purely a function of its parameters

Assuming that states and temporary tiddlers keep representation in sync with the store, it is very much so, at least for the very purpose for which it was created: to read (and only read) the inner contents of the calc widget, being some calculated output.

Should the widget output change, then precisely due to a change of tiddlers in the store that affect the computation, hence the very need to also update this macro's output... which works when used as an element attribute, but not standalone, which is ok.

you can check whether the document is TW's fakedom implementation

Thanks, I have modified the code accordingly, and added basic getElementById() support to fakedom.

@Jermolene: Shall I make a PR with respect to getElementById() for fakedom?

@Jermolene
Copy link

Assuming that states and temporary tiddlers keep representation in sync with the store, it is very much so

No, it is not. The output of the macro depends on the DOM which, in the examples you are talking about, depends on the tiddler store. So, the output of the macro is dependent upon the state of the tiddler store (as well as it's parameters). That means that it conflicts with the refresh mechanism.

Thanks, I have modified the code accordingly, and added basic getElementById() support to fakedom.

The way that you've copied the code makes it very hard to use GitHub to see what changes have been made to the underlying core file. I wouldn't be keen to accept getElementById() into the core fakedom implementation because I don't think it's at best not generally useful and at worst rather dangerous. So I'd advise monkey-patching your modification into the core fakedom implementation, rather than forking it.

@tobibeer
Copy link
Owner Author

@Jermolene,

So, the output of the macro is dependent upon the state of the tiddler store (as well as it's parameters).

This statement appears true for every single macro. (How) Is it not?

The way that you've copied the code makes it very hard to use GitHub to see what changes have been made to the underlying core file.

I know, hence my question for a PR. I'll make one just to show the diff.

I wouldn't be keen to accept getElementById() into the core fakedom implementation because I don't think it's at best not generally useful and at worst rather dangerous.

That I'll leave for you to decide. I would not know what could be dangerous about indexing ids.

So I'd advise monkey-patching your modification into the core fakedom implementation, rather than forking it.

You mean by overwriting it the way I do or via some other means of "extending" it? (Would you have a few lines of (pseudo-)code on how to do that?)

@tobibeer
Copy link
Owner Author

I made a PR to show the diff for a basic getElementById() support in fakedom:

TiddlyWiki/TiddlyWiki5#2095

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