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

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

Open
tomchiverton opened this issue Feb 22, 2024 · 8 comments

Comments

@tomchiverton
Copy link
Contributor

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
Copy link
Owner

atuttle commented Aug 13, 2024

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
Copy link
Contributor Author

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
Copy link
Owner

atuttle commented Aug 13, 2024

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
Copy link
Owner

atuttle commented Aug 14, 2024

@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
Copy link
Owner

atuttle commented Aug 19, 2024

@tomchiverton were you able to try out my branch?

@tomchiverton
Copy link
Contributor Author

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
Copy link
Contributor Author

Make that next week, sorry

@tomchiverton
Copy link
Contributor Author

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 ?

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