Skip to content
This repository was archived by the owner on Aug 4, 2021. It is now read-only.

Better handling of whitespace in serialization - #31

Merged
lukastaegert merged 4 commits into
masterfrom
esm-serialize-whitespace
May 11, 2018
Merged

Better handling of whitespace in serialization#31
lukastaegert merged 4 commits into
masterfrom
esm-serialize-whitespace

Conversation

@guybedford

Copy link
Copy Markdown
Contributor

Yes, I rewrote the wheel, but it's in the name of supporting custom types and non-unnecessary quotes, and I really couldn't find anything that did everything we need here.

* Update tested node versions and remove 0.12
* Update linting and run eslint --fix on commit
* Use correct check to serialize NaN
@lukastaegert

Copy link
Copy Markdown
Member

Ok, I've added a few commits here myself as I wanted to fix the Node 0.12 test issue. For which I found the best solution was to stop testing on Node 0.12 but instead align with rollup in testing Node 4, 6, 8 (and 10).

While doing this, I decided to add a prepare script and fix the long ignored linting to actually run (with the --fix option to auto-correct annoying white-space issues) on commit.

@lukastaegert

Copy link
Copy Markdown
Member

There was also in issue with serializing NaN that actually the linting discovered (NaN !== NaN...). I've added this to the test and fixed it.

@lukastaegert
lukastaegert merged commit a472802 into master May 11, 2018
@guybedford

Copy link
Copy Markdown
Contributor Author

Thanks for checking that and adding the changes!

The reason we can't use JSON.stringify is for serialization of dates and regexes needed for Toml or Yaml I think, so nice to get a single implementation that covers both. Then also useful to have legal keys unquoted for brevity.

@lukastaegert

Copy link
Copy Markdown
Member

Then also useful to have legal keys unquoted for brevity

That does indeed look very nice!

@lukastaegert
lukastaegert deleted the esm-serialize-whitespace branch May 11, 2018 14:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants