add React JSX filter #582

Merged
merged 1 commit into from Feb 8, 2015

Conversation

Projects
None yet
4 participants
Contributor

shieldo commented Mar 1, 2014

No description provided.

@stof stof commented on an outdated diff Mar 4, 2014

src/Assetic/Filter/ReactJsxFilter.php
+ $pb = $this->createProcessBuilder($this->nodeBin
+ ? array($this->nodeBin, $this->jsxBin)
+ : array($this->jsxBin));
+
+ $pb->add($inputDir);
+ $pb->add($outputDir);
+
+ $proc = $pb->getProcess();
+ $code = $proc->run();
+ unlink($input);
+
+ if (0 !== $code) {
+ throw FilterException::fromProcess($proc)->setInput($asset->getContent());
+ }
+
+ $file = new \SplFileInfo($input);
@stof

stof Mar 4, 2014

Collaborator

This looks weird to me given that you unlink the file a few lines above

@stof stof commented on an outdated diff Mar 4, 2014

src/Assetic/Filter/ReactJsxFilter.php
+
+ $pb->add($inputDir);
+ $pb->add($outputDir);
+
+ $proc = $pb->getProcess();
+ $code = $proc->run();
+ unlink($input);
+
+ if (0 !== $code) {
+ throw FilterException::fromProcess($proc)->setInput($asset->getContent());
+ }
+
+ $file = new \SplFileInfo($input);
+ $filename = $file->getFilename();
+ $output = file_get_contents($outputDir . '/' . $filename);
+ $asset->setContent($output);
@stof

stof Mar 4, 2014

Collaborator

you should delete the output file as well

Contributor

shieldo commented Mar 4, 2014

Thanks, you're right - I'll fix this up so the files are cleaned up properly.

Contributor

shieldo commented Mar 4, 2014

Now fixed up.

@stof stof commented on an outdated diff Mar 4, 2014

src/Assetic/Filter/ReactJsxFilter.php
+ $outputDir = $this->createTempDir();
+ $inputMinusExtension = tempnam($inputDir, '');
+ $inputFilePath = $inputMinusExtension . '.js';
+ file_put_contents($inputFilePath, $asset->getContent());
+
+ $pb = $this->createProcessBuilder($this->nodeBin
+ ? array($this->nodeBin, $this->jsxBin)
+ : array($this->jsxBin));
+
+ $pb->add($inputDir)->add($outputDir)->add('--no-cache-dir');
+
+ $proc = $pb->getProcess();
+ $code = $proc->run();
+
+ if (0 !== $code) {
+ throw FilterException::fromProcess($proc)->setInput($asset->getContent());
@stof

stof Mar 4, 2014

Collaborator

you need to remove the temporary files before throwing the exception

Contributor

shieldo commented Mar 4, 2014

Yes - reordered now.

@kriswallsmith kriswallsmith commented on an outdated diff Apr 5, 2014

src/Assetic/Filter/ReactJsxFilter.php
+{
+ /**
+ * @var string
+ */
+ private $jsxBin;
+
+ /**
+ * @var string
+ */
+ private $nodeBin;
+
+ /**
+ * @param string $jsxBin
+ * @param string $nodeBin
+ */
+ public function __construct($jsxBin = 'usr/bin/jsx', $nodeBin = null)
@kriswallsmith

kriswallsmith Apr 5, 2014

Owner

The default is missing a leading slash.

@kriswallsmith kriswallsmith commented on an outdated diff Apr 5, 2014

src/Assetic/Filter/ReactJsxFilter.php
+ }
+
+ /**
+ * Filters an asset just before it's dumped.
+ *
+ * @param AssetInterface $asset An asset
+ */
+ public function filterDump(AssetInterface $asset)
+ {
+ }
+
+ private function createTempDir()
+ {
+ $dirName = tempnam(sys_get_temp_dir(), 'assetic_react_jsx');
+ if (file_exists($dirName)) {
+ unlink($dirName);
@kriswallsmith

kriswallsmith Apr 5, 2014

Owner

This will fail if there are files inside the directory.

rbhalla commented Aug 5, 2014

Any movement on this PR? Would be great to see jsx support in assetic

Contributor

shieldo commented Aug 5, 2014

Caught napping! I'll see to this shortly.

@ghost

ghost commented Sep 11, 2014

Any movement on this PR? Would be great to see jsx support in assetic.
In the meantime, I use a make file

Contributor

shieldo commented Sep 11, 2014

Will look at it in the next week

Collaborator

stof commented Sep 12, 2014

please also rebase your PR as it conflicts with master

Collaborator

stof commented Oct 14, 2014

@shieldo ping

Contributor

shieldo commented Dec 7, 2014

Sorry it's taken so long, but I've now corrected these issues.

@kriswallsmith kriswallsmith merged commit 4c90c1f into kriswallsmith:master Feb 8, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
Owner

kriswallsmith commented Feb 8, 2015

Thanks, all!

Contributor

shieldo commented Feb 8, 2015

Thanks @kriswallsmith for merging and for your subsequent clean-up commits!

shieldo deleted the shieldo:jsx_filter branch Feb 9, 2015

shieldo restored the shieldo:jsx_filter branch Feb 9, 2015

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