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

Add jsform #58

Merged
merged 15 commits into from
Jan 4, 2021
Merged

Add jsform #58

merged 15 commits into from
Jan 4, 2021

Conversation

juancarlospaco
Copy link
Contributor

@juancarlospaco juancarlospaco commented Dec 15, 2020

This was referenced Dec 15, 2020
@juancarlospaco juancarlospaco mentioned this pull request Dec 25, 2020
@timotheecour
Copy link
Member

timotheecour commented Jan 3, 2021

how about:

func newFormData*(): FormData =
  when defined(nodejs):
    asm """
    var tmp = require("form-data")
    `result` = new tmp()
    """
  else:
    asm "`result` = new FormData()"

...

runnableExamples:
  from strutils import startsWith
  if defined(fusionJsFormdataTests):
    let data: FormData = newFormData()
    when defined(nodejs):
      data.add "key0", "value0"
      data.add "key1", "value1"
      var ret: cstring
      asm """`ret` = `data`.getHeaders()["content-type"]"""
      doAssert ($ret).startsWith """multipart/form-data; boundary=""", $ret
    else:
      data["key0".cstring] = "value0".cstring
      data.add("key1".cstring, "value1".cstring)
      data.delete("key1".cstring)
      doAssert data.hasKey("key0".cstring)
      doAssert data["key0".cstring] == "value0".cstring

tested with:

npm i form-data
nim doc -b:js --doccmd:'-d:fusionJsFormdataTests -d:nodejs' src/fusion/js/jsformdata.nim

note:
there's no set in that nodejs package apparently, see form-data/form-data#488

EDIT

ignore this comment, looks like require("form-data") is too different or at least this could be done in future work; IMO the better way would be to fix this: timotheecour/Nim#197 (in future work)

@timotheecour timotheecour requested a review from ringabout January 4, 2021 00:42
Copy link
Member

@ringabout ringabout left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after addressing the last comment

Co-authored-by: flywind <43030857+xflywind@users.noreply.github.com>
@timotheecour timotheecour merged commit 7b900a3 into nim-lang:master Jan 4, 2021
@juancarlospaco juancarlospaco deleted the js3 branch January 4, 2021 20:38
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

Successfully merging this pull request may close these issues.

3 participants