[PR] add typed input for filename prop on native file(in) node

Core Development Feature Requests
1

Hi,

i have fork node and extend file / file in node on filename properties for add choice of input type (msg, str, jsonata, ...)

Default it's str type with "" value, then old confidg node won't be impacted by this small change.

BTW it's my first fork and PR, maybe i don't doing thing right :blush:

Thank for time past in reply :slight_smile:

2

The normal method when having an idea for an enhancement to core nodes is to talk about it first and get agreement that its a good idea before issuing a PR :slight_smile:

3

Yep, i havent create a PR yet, before i open this topic :slight_smile:

1 Like
4

If I understand - you are replacing the textbox for filename with a typedInput?

What options are you intending to include?
string, msg, flow, global, jsonata all certainly make sense.

+1 from me as its a pet peeve of mine - having to add change nodes before a node simply to move a property into the payload or filename field.

So long as it is 100% compatible, I cant see why this wouldn't be accepted. (though I might be under-thinking this)

Fingers crossed you get the OK. :crossed_fingers:

5

You understand ! It's crazy to have 2 nodes, one for fill filename and other to perform file operation !

I have include all type (even env var)

The point it's 100% compatible, on my test everything it s ok but... we need more test...

6

it's not crazy - it's one of the main tenets of Node-RED - nodes should do their thing and do it well - moving properties around is the function of the change node - reading files is the function of the file node.

There is certainly no need to include all types - don't need boolean, number, buffer, etc.

Does this also apply to the file out node ? Note: there is a performance penalty if you use anything other than a fixed value as it has to close the file every write - just in case the next filename value is different.

7

Haha no it's not crazy but it is (to me anyhoo) a bit annoying when we have the fabulous typedinput at our disposal.

(IMHO, I wish all nodes which work on input msg parameters made use of typedinput - it's a tremendous feature of node-red)

In the interest of diplomacy, if the OP met the following conditions (plus any you stipulate), would you consider the PR Dave?

  • typedinput supports string, flow, global, jasonata, env (that's about all I think it needs)
  • by default, string type is set
  • both file in and file out node are done the same
  • 100% compatibly with existing flows

Ps. I fully expect I might be missing something so please advise.

8

Note I didn't say no ... just need to think about it a bit first (as we are doing)...

so yes - default must be string - and if that is blank it must still look for msg.filename (as now).
Will also need tests added to test/nodes/core/storage/10-file_spec.js to exercise new functionallity.

1 Like
9

I missed that one dammit. Good to see you are as annoyingly thorough as always. :wink:

10

well there is a test for that already so it should have found it...

11

Sorry for the wrord 'crazy"... I m a frog legs eater, and i m not confortable in English...

Then if you forgive me, i understand very well the atomic philosophy of a node, but i think typed input are for this probleme (easily retrieve object attach on msg)

Yep, all type are not necessary and you listed those i used. Default type are str, for compatibilty and if field is blank we look on msg.filename. Change made on both node.

In fact there 2 lines change in .js, would you like that i copy here the lines ?

12

No problem, I'd like a pull request with tests please, so we can check it and hopefully merge it.

13

Sorry i haven't yet good practice.

What you mean by test ? screenshot ? or test like unit test ?

14

unit tests - as in add tests to https://github.com/node-red/node-red/blob/master/test/nodes/core/storage/10-file_spec.js

1 Like
15

This topic was automatically closed 60 days after the last reply. New replies are no longer allowed.