expressjs / express

Fast, unopinionated, minimalist web framework for node.
https://expressjs.com
MIT License
65.62k stars 16.21k forks source link

app.delete() raising syntax errors #1750

Closed ghost closed 11 years ago

ghost commented 11 years ago

'delete' being a reserved keyword in Javascript, app.delete() is raising syntax errors in IDE like Nodeclipse and the like.

'delete' is the antonym keyword for 'new': nobody would ever use any construct like app.new() in express - just to point out how deep this mistake is to me.

A much better syntax would be app.remove() as 'remove' is not a reserved keyword - what a language reserved keywords are about if they are just ignored so lightly?

Is it still possible to address such an issue or is this 'delete' construct a legacy forever?

tj commented 11 years ago

app.del() is available, it's not a syntax error though, those IDEs are just outdated, you can use reserved words as properties

rlidwka commented 11 years ago

@nodeleaf

'delete' is the antonym keyword for 'new': nobody would ever use any construct like app.new() in express

> var x = {}
undefined
> x.new = 1
1
> x.delete = 1
1
> x
{ new: 1, delete: 1 }

It's a perfectly valid javascript code. You don't see "app.new()" only because express doesn't provide "new" function.

ghost commented 11 years ago

@rlidwka Thank you for teaching me the bottom of Javascript language but object's properties and a framework APIs like express are not on same level: app.del() is much better practice to me.

rlidwka commented 11 years ago

object's properties and a framework APIs like express are not on same level: app.del() is much better practice to me.

When the same thing is named differently in two places, it's usually a source of bugs and confusion. I recently debugged a few nice modules where "err.msg" and "err.message" were used at the same time in different contexts. It worked, but changing it wasn't really a pleasant experience.

There's no method named "DEL" in HTTP spec. That's reason enough to deprecate app.del() method.

ghost commented 11 years ago

@rlidwka

app.delete() performs HTTP DELETE requests like app.get() or app.post() performs GET or POST requests - I follow you in this. But neither 'get' or 'post' are reserved keywords in Javascript...

Your err.msg/message example is interesting but this is object properties again and my conclusions are different: I don't know why express provides 2 different methods to perform one single task - app.delete() + app.del() - but I suppose I was not the only one to get troubles with the former that fails syntax validation in Eclipse based IDEs.

I won't buy what was written above about IDEs - they are not obsolete by nature but their syntax validators are built to follow rules - and first of all misuses of a language keywords...

Validators might be too strict sometimes - var obj = { delete: 1 }; would raise an error in nodeclipse - but they can also help a lot to debug code or organize a project.

In conclusion - the language is reserving keywords, the HTTP protocol is supporting several request methods and we have a conflict on 'delete' that's used by both of them - therefore by express.

Well - I never said this was a simple issue but do we really have to make a choice between express and nodeclipse? I'm afraid I need both...

sorribas commented 11 years ago

They are outdated as of EcmaScript 5. It wasn't legal before but it is now, so this IDEs are validating for pre ES5 JavaScript VMs. Since we're using Node.js and we know node's vm (v8), and we know that it supports reserved words as property names, I don't see a problem with using app.delete.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Reserved_Words

Read the "Reserved words usage" part of this page,.

ghost commented 11 years ago

@sorribas

I've got it loud and clear ( finally... ) - nodeclipse validators being probably based on ES3 its validators are obsolete since v8 (therefore nodejs) is now based on ES5: you've marked a point here.

Since express is routing HTTP methods by name, app.delete() is legitimate per ES5 therefore nodeclipse should NOT raise any error about app.delete() in express or for object proprery like in var obj = { delete: 1 };

Let's see what nodeclipse team will have to answer about this one...