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

Stringifier Ignores Manually Added Func Child Nodes #69

Closed
jonathantneal opened this issue Mar 5, 2019 · 5 comments · Fixed by #73
Closed

Stringifier Ignores Manually Added Func Child Nodes #69

jonathantneal opened this issue Mar 5, 2019 · 5 comments · Fixed by #73

Comments

@jonathantneal
Copy link
Collaborator

  • Webpack Version: N/A
  • Operating System (or Browser): N/A
  • Node Version: N/A
  • postcss-values-parser Version: 3.0.1

How Do We Reproduce?

Insert a new node into an AST.

import valuesParser from 'postcss-values-parser';
import Punctuation from 'postcss-values-parser/lib/nodes/Punctuation';

const astValue = valuesParser.parse('rgb(100% 100% 100%)');

astValue.nodes[0].nodes.splice(1, 0, new Punctuation({ value: ',' }));
astValue.nodes[0].nodes.splice(2, 0, new Punctuation({ value: ',' }));

console.log({
	punctuation: valuesParser.nodeToString(astValue.nodes[0].nodes[1]),
	root: valuesParser.nodeToString(astValue)
});

Expected Behavior

Commas are added.

{
  punctuation: ',\n',
  root: 'rgb(100%, 100%, 100%)'
}

Actual Behavior

Commas are not added.

{
  punctuation: ',\n',
  root: 'rgb(100% 100% 100%)'
}
@shellscape
Copy link
Owner

Very strange. We're not deviating or overwriting how PostCSS's nodes behave in that regard.

@jonathantneal
Copy link
Collaborator Author

The issue is that Func is stringified by its params and not by its nodes.

astValue.nodes[0].params = 'NODES ARE IGNORED';

console.log({
	root: valuesParser.nodeToString(astValue) // "rgbNODES ARE IGNORED"
});

@shellscape shellscape changed the title Cannot insert a new Node Stringifier Ignores Manually Added Func Child Nodes Mar 5, 2019
@shellscape
Copy link
Owner

Yeah, here's the culprit:

this.builder(node.name + node.params, node, 'start');

@jonathantneal
Copy link
Collaborator Author

As you intend it, is params to be a sugary getter/setter (i.e. parse/nodeToString) of nodes, or should params be available at all?

@shellscape
Copy link
Owner

It was intended to give a snapshot of the function sans name. This was 100% a lack of foresight on my part. Currently working on a fix for that.

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

Successfully merging a pull request may close this issue.

2 participants