Deprecate implicit text-only for script and style

[ForbesLindesay]

closes #875

If we can remove this feature (probably in 0.32.0) then it will simplify a lot of code and significantly reduce the amount you have to learn when you're new to node.

The first step to removing it is to deprecate it and discourage people from using it, so this pull request logs a warning whenever someone relies on this feature. Dealing with the fallout from this will also help to gauge how many people we're going to annoy by removing this feature.

Crucially, once we actually remove the feature altogether, we will be able to write templates that do things like:

script
  import foo.js

which would be enormously useful.

[ForbesLindesay]

I will merge this if there are no complaints by 2013-05-25T12:00:00Z

I realize this will be a pain for some people, I'll see about creating an automated tool to help fix this for people.

[ForbesLindesay]

We can reduce pain now by encouraging people to use:

npm install fix-jade -g
fix-jade

which will automatically make the necessary change for all jade files in a directory (recursively).

[vendethiel]

thanks for what you do :-)

[stuartpb]

I think this change should be accompanied by a bot that files a pull request on all Node projects using Jade with the fix-jade output, like the bot somebody wrote to update all projects using the 'sys' module when it changed to 'util' in node 0.3.0 (praised at a Seattle Node meetup a few months back with "now THAT is how you introduce breaking changes!")

Granted that was 2.5 years and a few orders of magnitude of Node projects on Github ago, so it may not be as feasible now as it was then.

[ForbesLindesay]

@stuartpb that would be awesome, the question is how do we find them? I don't think the GitHub search API is sufficient. If people want to submit pull requests into fix-jade then hopefully we can start running it as a bot to submit pull requests. Unfortunately even once you have a repository there's a fair bit of work to be done in order to make the pull requests with the GitHub API.

I'm going to merge this now. We can encourage people to run fix-jade themselves, which is not very time consuming to do, and if it seems to be a problem we can work on creating a bot.

[stuartpb]

@ForbesLindesay I don't know how, but I do know it's been done before (the guy who wrote the bot was at the aforementioned Seattle Node meetup: unfortunately, I didn't catch his name). Step one to figuring out how such a thing would be accomplished would probably be finding one of said bot's pull requests and finding the approach / author from there.

[stuartpb]

Also, how isn't the Github search API sufficient? https://github.com/search?q=extension%3Ajade

[ForbesLindesay]

The problem with that is that it only provides the first 100 pages, and there's no way of getting more. That means we'd only get coverage of about 10% of all repos with jade files in them.

I've now written some code that retrieves a list of all repositories that are owned by someone who's starred jade. That seems like a good start.

[stuartpb]

Argh! Github's public-facing search API has different capabilities than their search page? That's regrettable. I'll see what can be done wrt that.

In the meantime I'd also add all the repositories of modules on npmjs.org that list jade as a dependency (if that isn't a strict subset of repos by users starring jade).

[ForbesLindesay]

That's not a bad suggestion, but most of the users of jade probably aren't in npm anyway as they will be the final finished applications.

It's not that their search API provides any less functionality than their search page. It's that neither provides the ability to run a search with more than 100 pages of results.

[ForbesLindesay]

@jade-bot is go!!!

[ForbesLindesay]

It's actually a node.js based system using express. I then use stop to turn it into a static site to host on gh-pages. Essentially it just runs the express app and then downloads all the content by parsing HTML for href and src attributes.

The source for the website is here.

To install:

npm install

To run at http://localhost:3000/

node src/server.js

To build static site to serve on gh-pages:

npm run build

Stop is configured via this file which is the only file that's specific to generating static sites.

Yes, the readme could have much nicer API docs if it didn't have quite so much stuff in it. It's way way too long to be a single page without a sticky table of contents that indicates where you are in the document.

[elisee]

What's the proper syntax for updating script!= javascript? Is it script!= javascript.?

I tried it because that's what the jade-bot offered to merge into express-expose but I get a non-sensical error when parsing my Jade template on the last line of my template:

 Line 39: Unexpected token )
  at throwError (...\node_modules\jade\node_modules\with\node_modules\lexical-scope\node_modules\astw\node_modules\esprima\esprima.js:1156:21)

Is jade-bot/express-expose@6b41eda wrong or am I hitting an unrelated bug?

[vendethiel]

it's wrong

[stuartpb]

I think the bot is wrong in that script!= javascript should work without change (since that's how all the other non-text-only tags work, and the only thing this change does is make script and style work like the other tags).

[elisee]

@stuartpb I'm still getting a deprecation warning though, so either the warning itself is spurious (which might well be the case) or there is some change to make.

[ForbesLindesay]

Hi @elisee sorry about this, it's a bug in the current version of jade and it's entirely my fault. We don't have any test cases for:

script foo()

or

script!= code

so I didn't notice the deprecation warning showing up when it shouldn't.

I'll fix it in the next version of Jade but I'm afraid I have exams for the next couple of days. I'll get right on it Friday.

[tsgautier]

I'm getting this warning now - but I don't even know in which file is the offending code. It would be helpful if the warning could point out where the deprecated usage exists so I could find and fix it.

[elisee]

@ForbesLindesay ok thanks for the details, good to know it's nothing to worry about and that I don't have to make any changes.

[ForbesLindesay]

@tsgautier

just searching for script or style should find you the files that might have offending tags. Alternatively:

npm install fix-jade -g
fix-jade

will recursively look for and attempt to automatically fix any files with problems in (and list all the files that had problems). I wouldn't recommend using it without version control though in case it gets something wrong.

@elisee

No problem, obviously you would need to fix it if you had something like:

script
  foo()

You would then need to change it to:

script.
  foo()

[YukonSaint]

Can you tell me how to convert the following code:

script(type='text/javascript', src='/assets/js/vendor/underscore-min.js')

Or do I need to convert it at all?

FYI - I realize that "Implicit textOnly for script and style is deprecated. Use script. or style. instead." might make plenty of sense to Jade contributors and experienced Jade developers. But for someone who is fairly new to Jade it's very confusing. One or two quick examples in the release notes would be an enormous help.

[stuartpb]

@mattsavino: You don't need to convert it, and indeed you shouldn't even get a deprecation warning for script tags with src attributes.

[stuartpb]

As for a different warning for script and src, maybe this:

"(file): lines (first)-(last): Using text content under a (script / style) element without . is deprecated. Use (script. / style.) instead. (script/style) will be changed to interpret content the same as all other tags in Jade 0.32.0."

[stuartpb]

Alternately, if the Jade site is beefed up, the change could be explained in detail on a documentation page and the deprecation warning could just be changed to a hyperlink like "See http://jade-lang.org/textonlytags.html".

[stuartpb]

I don't have much experience with the Jade parser, but would #1063 tighten up the deprecation context correctly?

I removed the script+src attribute check because that should really have its own warning, since this case is prohibited by the HTML5 spec.

[ForbesLindesay]

OK, I like that new suggestion for a beefed up warning message and I'm happy to have that.

I think the beefing up of the jade site might take slightly more time. I don't want to have random pages that are only used as a place to link to from a deprecation warning. Also, urls won't end with file extensions so it would be something like jade0lang.org/textonlytags/

I agree, I was using the src check to try and catch all the scripts that didn't have blocks, I just didn't account for those with inline blocks. Your solution is much neater.

I quite like the idea of rejecting scripts that have bodies and an src attribute as a compilation error at some future point, but it should be left to further down the line.

[stuartpb]

What about http://jade-lang.org/breaking-changes#implicit-textonly ?

[ForbesLindesay]

tbh the easiest thing is probably just to write a blog post on the subject. I'll sort it out at some point.

[stuartpb]

That's kind of what I was thinking http://jade-lang.org/breaking-changes would be - like a doc page that lists breaking changes that have been made in Jade, with each change getting its own blog-post-like section.

[stuartpb]

Does this change also mean the include filter for .js files that wrap them in <script> tags is going to be dropped?

[ForbesLindesay]

It's not something I plan on doing in the near future, but I do plan on making filters explicit in some way so that it'll be possible to include a file and just get the plain text of a markdown doc or js file or whatever. That will be the next major breaking change.

I think encouraging people then to switch from what would be something like:

include js:foo/bar.js

to something like

script
  include foo/bar.js

may be a good idea. I don't think this would need to include deprecation warngings right away though.

[stuartpb]

Wouldn't include:js make more sense, since filters have traditionally been colon-prefixed (and filenames can technically have colons in them)?

[ForbesLindesay]

Yes, probably, I haven't settled on any final syntax yet. I'll open an issue so it can be discussed before anything is decided.

[jescalan]

Hey @ForbesLindesay - can we get at least which file/line the error is coming from in the deprecation warning? Just upgraded on a big project and getting shit tons of these warnings, but no idea where to find them or change them. I'm aware of projects like fix-jade, but I'd like to be able to just jump in and make these changes myself.

[ForbesLindesay]

Yes, patch welcome