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

Dash (-) not supported in resource name #359

Open
praihan opened this issue Jan 12, 2018 · 10 comments
Open

Dash (-) not supported in resource name #359

praihan opened this issue Jan 12, 2018 · 10 comments

Comments

@praihan
Copy link

praihan commented Jan 12, 2018

The resource and attribute names are checked against the regex /^[A-Za-z0-9_]*$/. Is it possible to also support -, i.e. resource-name-is-like-this format? It could just be as easy as changing it to /^[A-Za-z0-9_\-]*$/.

@pmcnr-hx
Copy link
Contributor

I will have to check the spec to see if - is allowed. I'll come back to you on this one.

@praihan
Copy link
Author

praihan commented Jan 16, 2018

We see here:

The values of type members MUST adhere to the same constraints as member names.

Which says

The following “globally allowed characters” MAY be used anywhere in a member name:

  • U+0061 to U+007A, “a-z”
  • U+0041 to U+005A, “A-Z”
  • U+0030 to U+0039, “0-9”
  • U+0080 and above (non-ASCII Unicode characters; not recommended, not URL safe)

Additionally, the following characters are allowed in member names, except as the first or last character:

  • U+002D HYPHEN-MINUS, “-“
  • U+005F LOW LINE, “_”
  • U+0020 SPACE, “ “ (not recommended, not URL safe)

The requirements in the spec are pretty clear and should be simple to extract into a function instead of a regex test. Let me know what you think.

@pmcnr-hx
Copy link
Contributor

You are correct @prshreshtha. You're welcome to submit a PR for review and eventual merging.

@praihan
Copy link
Author

praihan commented Jan 20, 2018

Sounds good. I'll make one soon.

@praihan
Copy link
Author

praihan commented Jan 23, 2018

It looks like GraphQL does not support dasherized field names. Any thoughts @pmcnr-hx? The same issue occurs with resources that start with a number.

@paparomeo
Copy link
Contributor

paparomeo commented Jan 24, 2018

Ah, yes... I seemed to recall there was a reason not prevent dashes. One options is to replace the - with _ on the fly. The other one is to disable GraphQL support if you prefer to use just use JSON:API with - on the names. I don't think GraphQL support should prevent us from fully supporting the JSON:API spec. Which option do you prefer?

@praihan
Copy link
Author

praihan commented Jan 24, 2018

@paparomeo there are trade-offs here. I don't think replacing - with _ will work without breaking the API since my-resource and my_resource will collide. I think the ability to disable GraphQL support to take full advantage of JSON:API is the way to go and document the stricter requirements when using GraphQL. This definitely needs some research.

@paparomeo
Copy link
Contributor

Thanks for the investigation @prshreshtha. I should have some time soon to look into adding support for disabling GraphQL.

@queenvictoria
Copy link

queenvictoria commented Mar 15, 2018

You can already do part of this. And there is an issue here for the entire task. #256

From my current implementation

jsonApi.setConfig({
  port: PORT,
  base: `api/${API_ENV}/v${API_VERSION}`,
  graphiql: true, // <-----------
  swagger: {
    title: "JSON:API Server",
    version: "0.1.0",
    description: "",
    contact: {
      name: "API Contact",
      email: "[email protected]",
      url: "example.com"
    },
    license: {
    }
  },
  meta: function(request) {
    return {
      timestamp: parseInt(new Date().getTime()/1000),
      schema: `api/${API_ENV}/v${API_VERSION}/swagger.json`,
    };
  }
});

@praihan
Copy link
Author

praihan commented Mar 15, 2018

@queenvictoria You are correct that #256 does cover part of this issue. It is a pre-requisite to supporting dasherized names. But I don't see any work done for it.

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

4 participants