flibbles / tw5-uglify

Compress TiddlyWiki5 javascript and plugins
Other
18 stars 2 forks source link

Problems creating uglified tiddlywikicore.js file with TiddlyWiki version 5.3.0 #8

Closed simonbaird closed 1 year ago

simonbaird commented 1 year ago

Summary

Producing an uglified version of tiddlywikicore.js is buggy for the newly released TiddlyWiki 5.3.0.

Details

Here's how I'm creating an external core empty and uglified tiddlywikicore.js. The commands are run in the TiddlyWiki5 git repo with tw5-uglify checked out in an adjacent directory:

export TIDDLYWIKI_PLUGIN_PATH=../tw5-uglify

node tiddlywiki.js editions/empty --output output/external-core \
  --rendertiddler '$:/core/save/offline-external-js' 'empty.html' 'text/plain'

node tiddlywiki.js +plugins/plugins/uglify editions/empty --output output/external-core \
  --render '$:/core/templates/tiddlywiki5.js' '[[tiddlywikicore-]addsuffix<version>addsuffix[.js]]' 'text/plain'

ls -l output/external-core

If you open the generated empty.html file you can see the problem. There is <<>> showing up in many places instead of the expected content.

Looking at the core js file, there is text like Update <<>> plugins inside the $:/core plugin, instead of Update <<update-count>> plugins which I think is causing the problem. If you search for <<>> you can find many other examples. I guess somehow the uglify code is stripping out the text incorrectly.

Producing a non-uglified core js file like this is useful to compare the uglified and non-uglified output:

node tiddlywiki.js editions/empty --output output/external-core \
  --render '$:/core/templates/tiddlywiki5.js' '[[tiddlywikicore-]addsuffix<version>addsuffix[.normal]addsuffix[.js]]' 'text/plain'
Jermolene commented 1 year ago

Thanks @simonbaird I hope @flibbles can have a look at this.

flibbles commented 1 year ago

@Jermolene, I'm seeing that with v5.3.0, the macrocall parseTreeNode no longer has a "name" attribute specifying the name of the macrocall. (e.g. <<myMacro X>> would have node.name === "myMacro"). And now there's something called "$variable" filling up the attributes that seems to be taking its place?

What was the rational here? And wasn't this a backward-compatibility breaking change?

kookma commented 1 year ago

Not the wizard and not node.js version of Uglify work for me with TW 5.3.0

Jermolene commented 1 year ago

Hi @flibbles

@Jermolene, I'm seeing that with v5.3.0, the macrocall parseTreeNode no longer has a "name" attribute specifying the name of the macrocall. (e.g. <<myMacro X>> would have node.name === "myMacro"). And now there's something called "$variable" filling up the attributes that seems to be taking its place?

What was the rational here? And wasn't this a backward-compatibility breaking change?

In v5.3.0 we deprecated the macrocell widget, instead implementing the functionality via a new capability for the transclude widget to transclude the contents of a variable.

The rationale was consistency; previously we had two widgets that do almost the same thing slightly differently, with slightly different capabilities. Now we have a single widget to do the thing, with consistent options and capabilities.

Backwards compatibility is a subtle problem. None of the improvements that we make can be strictly 100% backwards compatible because they always change the behaviour of the system in some way. This particular change might impact any plugin that manipulates parse trees.

Anyhow, I think there's a deeper problem that we need to fix: parse tree nodes refer to widgets, rather than to semantic constructions. I think things would be more flexible if the parser nodes were semantic, with an extensible process for mapping particular parse tree node types to the equivalent widget (or widget tree fragment). Under this approach, a call like <<macro>> would generate a different parse tree than <$transclude $variable="macro"/>.

flibbles commented 1 year ago

Fixes for macrocalls are now in. Uglify should work fine for v5.3.0 now, even though there is no support for all the new pragma wikitext (Uglify should be good about ignoring unrecognized wikitext).

I'll add support for the new content eventually, but other plugins of mine are still broken as of v5.3.0, and they need attention first, and I need more time in my day, and I need to learn what all these new features are from v5.3.0, because I still don't know.

simonbaird commented 1 year ago

I'll give it a try with Tiddlyhost's external core json file soon.

simonbaird commented 1 year ago

Do you need to push to main or something? GitHub is saying https://github.com/flibbles/tw5-uglify/tree/v1.6.1 does not belong to any branch on this repository.

flibbles commented 1 year ago

Huh. That's weird. I always thought "git push --tags" would push the relevant branches too. Guess not.

Should work now.

flibbles commented 1 year ago

Hold up.

Looks like v5.3.0 has introduced some glitch with attribute parsing that uglify is exposing. In its current state, it won't suit your needs. I'll let you know once this is fixed, either in TW, or with uglify making a workaround.

flibbles commented 1 year ago

Okay. Looks like the "backtick" quotes are just a thing for attributes now, so Uglify will need to support them.

I'm torn between acknowledging that these substitution attributes are a good feature (because they are), and complaining about having more work dropped in my lap.

simonbaird commented 1 year ago

Thanks @flibbles, appreciate your work. I'll hold off.

For the record, this is not urgent from my perspective. Tiddlyhost has a non-uglified 5.3.0 core js file, and, while it is a little bigger, I don't think anyone is noticing. It's being cached forever and is likely downloaded as a gzip file to begin with, so the impact is not large.

I'd rather proceed cautiously than risk pushing something that could start breaking external core sites on Tiddlyhost.

flibbles commented 1 year ago

@simonbaird, should be all set now. TW v5.3.0 should be fully supported by uglify now, including all new syntax.

kookma commented 1 year ago

Version 1.7 works form me. Thank you @flibbles.