atuttle / Taffy

:candy: The REST Web Service framework for ColdFusion and Lucee
http://taffy.io
Other
226 stars 117 forks source link

Lucee 6 - queryToStruct is now a reserved word as it's a built in function, with different arguments #437

Open tomchiverton opened 6 months ago

tomchiverton commented 6 months ago

Probably the Taffy method should be renamed, or made available under a different name, and all the docs updated ?

Workaround, change every API resource from return representationOf( queryToStruct(... to return representationOf( this.queryToStruct(...

atuttle commented 3 weeks ago

Thanks for the tip, @tomchiverton! I'm happy to make whatever the appropriate changes are.

I'm surprised that queryToStruct has been added but queryToArray hasn't. If we're going to make this change, we should probably anticipate the addition of queryToArray to the language, too. It seems inevitable, and it only makes sense to keep the Taffy methods consistent with themselves.

I don't have a Lucee 6 env handy, don't have time to build one, and the version available on TryCF seems to be ~unusable at the moment.

I would prefer to not break backwards compatibility if possible. Does Lucee 6 throw an exception because of Taffy's queryToStruct method, or is there just a conflict preventing accessing the one you want to use?

I think an ideal solution would allow existing code to continue using queryToStruct while adding something else (qToStruct and qToArray?) for Lucee 6. I'd hate to break existing implementations over something so minor.

tomchiverton commented 3 weeks ago

Are you thinking Taffy could catch the exception caused by the argument mismatches? But the return rep(queryToStruct(... is inside the resource CFC so not sure how that would help - the framework can't get at the inner content ?

For CFC's that use the more modern "just return anything" it might be more possible?

atuttle commented 3 weeks ago

This is kind of what I was thinking:

<cffunction name="qToStruct">...</cffunction>

<cftry>
  <cffunction name="queryToStruct">
    <cfscript>
      return qToStruct(...);
    </cfscript>
  </cffunction>
  <cfcatch> <!--- fail silently ---> </cfcatch>
</cftry>
atuttle commented 3 weeks ago

@tomchiverton I've created a branch that I'm hoping fixes the problem. Please give it a try: https://github.com/atuttle/Taffy/tree/feat/qToStruct

It does the following:

  1. Rename the taffy-implementation of queryToStruct to qToStruct
  2. Check to see if queryToStruct is a built in function and if found, make no other changes
  3. However, if it's missing, it creates an alias for qToStruct named queryToStruct.
  4. Do the same thing for queryToArray for consistency.

see the diff here

This means that on Lucee 6, you would have the choice to use the built-in queryToStruct, or Taffy's qToStruct, and everywhere that doesn't implement queryToStruct would still have Taffy's implementation available with that name.

atuttle commented 2 weeks ago

@tomchiverton were you able to try out my branch?

tomchiverton commented 2 weeks ago

I might have a chance to do so today, but certainly this week.

We've migrated most of our code to use the this. workaround, as we're on Lucee 6 in production, so I think I'll need to undo that for a fair test.

tomchiverton commented 1 week ago

Make that next week, sorry

tomchiverton commented 1 week ago

This means that on Lucee 6, you would have the choice to use the built-in queryToStruct, or Taffy's qToStruct, and everywhere that doesn't implement queryToStruct would still have Taffy's implementation available with that name.

This branch doesn't seem to have any bad effects for our projects still using Lucee 5.x

All our Lucee 6.x projects have already moved to using "this.queryToStruct()" so it's kinda hard to tell the impact :)

I'd say merge the feature, then update the docs to refer to qToXxxx and note somewhere that it used to be queryToXxxx ?