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

JSX render won't allow webkitdirectory and directory to be used #3468

Closed
GaryYufei opened this issue Mar 20, 2015 · 36 comments
Closed

JSX render won't allow webkitdirectory and directory to be used #3468

GaryYufei opened this issue Mar 20, 2015 · 36 comments

Comments

@GaryYufei
Copy link

I tried to use webkitdirectory and directory in the input DOM. But I found that it is impossible to add these two label to the input DOM. Anyway to solve this problem?

@jimfb
Copy link
Contributor

jimfb commented Apr 1, 2015

We currently have a whitelist of supported attributes. We want to remove the whitelist and allow all attributes at some point (#140) but it's tricky because some components pass all attributes to their children.

Edit: Actually, changed my mind, we'll leave this one open to track the addition of webkitdirectory, mozdirectory, and directory until such time as #140 is fixed or these are added.

A #goodfirstbug might be to add these to the whitelist.

@jimfb jimfb closed this as completed Apr 1, 2015
@jimfb jimfb reopened this Apr 1, 2015
@jimfb
Copy link
Contributor

jimfb commented Apr 10, 2015

webkitdirectory was added by #3644.

@jimfb jimfb closed this as completed Apr 10, 2015
@jimfb jimfb reopened this Apr 10, 2015
@jimfb
Copy link
Contributor

jimfb commented Apr 10, 2015

Still need mozdirectory and directory.

@mikecheb
Copy link

mikecheb commented May 9, 2015

#3644 was reverted by @zpao. He makes a good point about camelcase.

Where should we continue this discussion? Here?

Currently I'm working around this by manipulating the DOM node directory in componentDidMount.

@mxstbr
Copy link
Contributor

mxstbr commented Feb 18, 2016

Any update on this? Taking a cue from #3644, I could submit a PR adding mozdirectory, directory, webkitdirectory and nwdirectory if that's still wanted. /cc @zpao

@zpao
Copy link
Member

zpao commented Feb 18, 2016

I think we're pretty unlikely to take it for the next release (very soon) and we'll hopefully get to a point before the next release after that where all attrs will work without the whitelist. So I'm not super inclined to spend any effort here at the moment.

@mxstbr
Copy link
Contributor

mxstbr commented Feb 18, 2016

Fair enough, I reckon this can be closed then!

@victorb
Copy link

victorb commented Aug 20, 2016

@zpao any news regarding being able to use mozdirectory and the rest with or without the whitelist?

@fa7ad
Copy link

fa7ad commented Sep 10, 2016

BUMP!

Is nwdirectory supported on 15.x?

@sassanh
Copy link
Contributor

sassanh commented Sep 10, 2016

Guys it's such a required feature. Is there any workaround for it?

@gaearon
Copy link
Collaborator

gaearon commented Sep 10, 2016

We intend to drop the whitelist and pass all attributes through, possibly in React 16.

@gaearon
Copy link
Collaborator

gaearon commented Sep 10, 2016

In the meantime you can do this in componentDidMount by putting a ref on the node, and calling setAttribute yourself.

@fa7ad
Copy link

fa7ad commented Sep 10, 2016

@gaearon I'm doing that right now, but that's just a dirty hack. Plus, going through the PRs I noticed one that suggests that we might be getting warnings about direct DOM manipulations in the future, so there's that 😟

@victorb
Copy link

victorb commented Sep 10, 2016

Thanks @gaearon for the update and possible workaround.

@sassanh
Copy link
Contributor

sassanh commented Sep 10, 2016

Yeah I did this:

    componentDidMount() {
        this.refs.x.directory = true;
        this.refs.x.webkitdirectory = true;
    }

but I just don't like these lines in my code.

@sassanh
Copy link
Contributor

sassanh commented Sep 10, 2016

Though of course we can wait till react 16 is out.

@sassanh
Copy link
Contributor

sassanh commented Sep 10, 2016

Setting the html tag attributes in componentDidMount doesn't always work. For example in my case I had something like this:

    componentDidMount() {
        this.refs.x.directory = true;
        this.refs.x.webkitdirectory = true;
    }

    render() {
        return this.context.pending
            ? <input ref='x' type='file'/>
            : <div>y</div>;
    }

To solve this I had to do this:

    componentDidMount() {
        if (this.refs.x) {
            this.refs.x.directory = true;
            this.refs.x.webkitdirectory = true;
        }
    }

    componentDidUpdate() {
        if (this.refs.x) {
            this.refs.x.directory = true;
            this.refs.x.webkitdirectory = true;
        }
    }

    render() {
        return this.context.pending
            ? <input ref='x' type='file'/>
            : <div>y</div>;
    }

I guess doing this every time componentDidUpdate triggers shouldn't have any overhead. I think browser ignores it if it's already set.

@fa7ad
Copy link

fa7ad commented Sep 10, 2016

@sassanh isn't string refs deprecated?

I'm doing it like,

...
render () {
  return (
    <input ref={i => this._inp = i} type='file' />
  )
}

componentDidMount () {
  this._inp.nwdirectory = true
}

As for your conditional you could just define an empty object called this._inp in the constructor, so it wouldn't do anything if the ref doesn't exist. The result would be just a property assignment for an unused object.

@aweary
Copy link
Contributor

aweary commented Sep 10, 2016

@fa7ad string refs are legacy, but not deprecated yet.

Although string refs are not deprecated, they are considered legacy, and will likely be deprecated at some point in the future. Callback refs are preferred.

Refs to Components

@fa7ad
Copy link

fa7ad commented Sep 10, 2016

@aweary sorry, missed that. Been a long time since I visited the site. Anyhow, since we know why this is a wontfix, can we close this?

@sassanh
Copy link
Contributor

sassanh commented Sep 10, 2016

@fa7ad actually I like the legacy string refs. They're great for simple use-cases like this and don't force you to define another variable on your class. I'm sure there are use-cases for new function refs but for simple use-cases like this I prefer string refs. I hope it'll never get deprecated.

@gaearon
Copy link
Collaborator

gaearon commented Sep 10, 2016

I'm doing that right now, but that's just a dirty hack. Plus, going through the PRs I noticed one that suggests that we might be getting warnings about direct DOM manipulations in the future, so there's that 😟

I’m not sure what you’re referring to, but touching DOM from lifecycle hooks was (and will be) allowed as long as you’re careful to not mess up the tree controlled by React.

Setting the html tag attributes in componentDidMount doesn't always work.

Yes, but you can fix this by using callback refs instead and doing the work in the ref callback.

    _addDirectory(node) {
      if (node) {
        node.directory = true;
        node.webkitdirectory = true;
      }
    }

    render() {
      return this.context.pending
        ? <input ref={node => this._addDirectory(node)} type='file'/>
        : <div>y</div>;
    }

They're great for simple use-cases like this and don't force you to define another variable on your class. I'm sure there are use-cases for new function refs but for simple use-cases like this I prefer string refs.

Ironically this is exactly the situation where callback refs do the job better 😄 . You don’t have to use them to define a variable on the class, you can do any work inside them (including adding attributes like I showed above).

I hope it'll never get deprecated.

They will.

@sassanh
Copy link
Contributor

sassanh commented Sep 10, 2016

Yeah, a good use-case for refs. But this is not the same use-case as above, to keep a reference to a dom element simply to do a manipulation on it later I still prefer string refs over storing it on a variable in class - though I hope I don't need to do that at all, maybe when react is version 1 someday we don't feel the need to use it.

@fa7ad
Copy link

fa7ad commented Sep 10, 2016

@sassanh react is version 15 right now, so... 😕

@sassanh
Copy link
Contributor

sassanh commented Sep 10, 2016

Oh I thought it's version .15. I searched react in google and went to https://facebook.github.io/react/ to check it out, then went to my package.json file and you're right it's version 15. My bad.

willzofsteel added a commit to willzofsteel/vigilant-giggle that referenced this issue Sep 21, 2016
- uses chrome webkitdirectory for uploading folders
- currently using componentWillMount to set webkitdirectory attribute
  due to how react treats this attribute, see link below

- facebook/react#3468
@gaearon
Copy link
Collaborator

gaearon commented Sep 8, 2017

This will be supported in React 16.

https://facebook.github.io/react/blog/2017/09/08/dom-attributes-in-react-16.html

@toddobryan
Copy link

toddobryan commented Oct 28, 2018

Is it just me, or is this still not supported in React 16? I'm on ^16.6.0 of React and ReactDOM and I'm having exactly the issues described here. And I don't think the workaround works in TypeScript.

@bheptinh
Copy link

bheptinh commented Dec 18, 2018

It works for me if I add empty string

<input directory="" webkitdirectory="" type="file" />

@softmarshmallow
Copy link

softmarshmallow commented Oct 7, 2020

It works for me if I add empty string

<input directory="" webkitdirectory="" type="file" />

this does not seems to be working..

Type '{ directory: string; webkitdirectory: string; type: string; }' is not assignable to type 'DetailedHTMLProps<InputHTMLAttributes<HTMLInputElement>, HTMLInputElement>'.
  Property 'directory' does not exist on type 'DetailedHTMLProps<InputHTMLAttributes<HTMLInputElement>'.ts(2322)

@fa7ad
Copy link

fa7ad commented Oct 7, 2020

@softmarshmallow this might have to do with the typescript typings rather than an issue in React itself. Last I checked, the empty string thing works in normal JavaScript just fine.

@qkreltms
Copy link

qkreltms commented Oct 22, 2020

@softmarshmallow I was able to fix this issue by doing this /* @ts-expect-error */
Note: it requires TS 3.9 and above

You should check this out: https://stackoverflow.com/questions/43618878/how-to-disable-a-ts-rule-for-a-specific-line

      <input
        type="file"
        id="dcmFolderSelector"
        multiple
        /* @ts-expect-error */
        directory=""
        webkitdirectory=""
        ref={dicomInputRef}
        onChange={onUploadHandler}
      />

@arnavzek
Copy link

I am still facing this issue. no error is thrown. It just doesn't work

@arnavzek
Copy link

In my case, it got fixed by removing styled-components

@manoharlalitech
Copy link

manoharlalitech commented Feb 7, 2022

declare module 'react' {
  interface InputHTMLAttributes<T> extends HTMLAttributes<T> {
    // extends React's HTMLAttributes
    directory?: string;
    webkitdirectory?: string;
  }
}

Use above code after your imports , then you can use both directory and webkitdirectory. its works in tsx and jsx files.

@bsides
Copy link

bsides commented Feb 10, 2022

declare module 'react' {
  interface InputHTMLAttributes<T> extends HTMLAttributes<T> {
    // extends React's HTMLAttributes
    directory?: string;
    webkitdirectory?: string;
  }
}

Use above code after your imports , then you can use both directory and webkitdirectory. its works in tsx and jsx files.

Nice one, but I believe the type would be directory?: boolean | undefined for each of them (webkitdirectory or mozdirectory too)

@f-o-w-l
Copy link

f-o-w-l commented Dec 15, 2022

declare module 'react' {
  interface InputHTMLAttributes<T> extends HTMLAttributes<T> {
    // extends React's HTMLAttributes
    directory?: string;
    webkitdirectory?: string;
  }
}

Use above code after your imports , then you can use both directory and webkitdirectory. its works in tsx and jsx files.

Nice one, but I believe the type would be directory?: boolean | undefined for each of them (webkitdirectory or mozdirectory too)

Nope, it doesn't work with React if you use a boolean. Who knows when they'll be added to the whitelist, or if React is even still using a whitelist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests