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

Enables jsx plugin in case jsx syntax is used in js files #2530

Merged
merged 7 commits into from Feb 8, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,3 @@
import * as Hyperapp from 'hyperapp'

module.exports = <div />;
@@ -0,0 +1,3 @@
{
"private": true
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The underlying problem this PR is trying to solve is that JSX in JS support fails when no dependencies are specified.

No dependencies are two-folded

  1. When there are no deps in package.json
  2. When there is no package.json

Unfortunately, I couldn't capture the second case with those tests as omitting package.json would modify package.json in other directories (the top-level package.json in integration-tests I think).

However, creation of package.json is tested on many other occasions, which would lead us back to case 1. So case 2. should work by implication (tested locally, is fine).

@@ -0,0 +1,3 @@
import * as Nerv from 'nervjs';

module.exports = <div />;
@@ -0,0 +1,3 @@
{
"private": true
}
@@ -0,0 +1,3 @@
const Preact = require('preact');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not using import here for the sake of testing require syntax as well.


module.exports = <div />;
@@ -0,0 +1,3 @@
{
"private": true
}
@@ -0,0 +1,3 @@
import * as React from 'react';

module.exports = <div />;
@@ -0,0 +1,3 @@
{
"private": true
}
89 changes: 89 additions & 0 deletions packages/core/integration-tests/test/javascript.js
Expand Up @@ -1343,6 +1343,28 @@ describe('javascript', function() {
assert(file.includes('React.createElement("div"'));
});

it('should support compiling JSX in JS files with React dependency even if React is not specified as dependency', async function() {
let originalPkg = await fs.readFile(
__dirname + '/integration/jsx-react-no-dep/package.json'
);

await bundle(
path.join(__dirname, '/integration/jsx-react-no-dep/index.js')
);

let file = await fs.readFile(
path.join(__dirname, '/dist/index.js'),
'utf8'
);

assert(file.includes('React.createElement("div"'));

await fs.writeFile(
__dirname + '/integration/jsx-react-no-dep/package.json',
originalPkg
);
});

it('should support compiling JSX in JS files with Preact dependency', async function() {
await bundle(path.join(__dirname, '/integration/jsx-preact/index.js'));

Expand All @@ -1353,6 +1375,28 @@ describe('javascript', function() {
assert(file.includes('h("div"'));
});

it('should support compiling JSX in JS files with Preact dependency even if Preact is not specified as dependency', async function() {
let originalPkg = await fs.readFile(
__dirname + '/integration/jsx-preact-no-dep/package.json'
);

await bundle(
path.join(__dirname, '/integration/jsx-preact-no-dep/index.js')
);

let file = await fs.readFile(
path.join(__dirname, '/dist/index.js'),
'utf8'
);

assert(file.includes('h("div"'));

await fs.writeFile(
__dirname + '/integration/jsx-preact-no-dep/package.json',
originalPkg
);
});

it('should support compiling JSX in JS files with Nerv dependency', async function() {
await bundle(path.join(__dirname, '/integration/jsx-nervjs/index.js'));

Expand All @@ -1363,14 +1407,59 @@ describe('javascript', function() {
assert(file.includes('Nerv.createElement("div"'));
});

it('should support compiling JSX in JS files with Nerv dependency even if Nerv is not specified as dependency', async function() {
let originalPkg = await fs.readFile(
__dirname + '/integration/jsx-nervjs-no-dep/package.json'
);

await bundle(
path.join(__dirname, '/integration/jsx-nervjs-no-dep/index.js')
);

let file = await fs.readFile(
path.join(__dirname, '/dist/index.js'),
'utf8'
);

assert(file.includes('Nerv.createElement("div"'));

await fs.writeFile(
__dirname + '/integration/jsx-nervjs-no-dep/package.json',
originalPkg
);
});

it('should support compiling JSX in JS files with Hyperapp dependency', async function() {
await bundle(path.join(__dirname, '/integration/jsx-hyperapp/index.js'));

let file = await fs.readFile(
path.join(__dirname, '/dist/index.js'),
'utf8'
);

assert(file.includes('h("div"'));
});

it('should support compiling JSX in JS files with Hyperapp dependency even if Hyperapp is not specified as dependency', async function() {
let originalPkg = await fs.readFile(
__dirname + '/integration/jsx-hyperapp-no-dep/package.json'
);

await bundle(
path.join(__dirname, '/integration/jsx-hyperapp-no-dep/index.js')
);

let file = await fs.readFile(
path.join(__dirname, '/dist/index.js'),
'utf8'
);

assert(file.includes('h("div"'));

await fs.writeFile(
__dirname + '/integration/jsx-hyperapp-no-dep/package.json',
originalPkg
);
});

it('should support optional dependencies in try...catch blocks', async function() {
Expand Down
24 changes: 24 additions & 0 deletions packages/core/parcel-bundler/src/transforms/babel/jsx.js
Expand Up @@ -12,6 +12,26 @@ const JSX_PRAGMA = {
hyperapp: 'h'
};

function createJSXRegexFor(dependency) {
// result looks like /from\s+[`"']react[`"']|require\([`"']react[`"']\)/
return new RegExp(
`from\\s+[\`"']${dependency}[\`"']|require\\([\`"']${dependency}[\`"']\\)`
);
}

/**
* Solves a use case when JSX is used in .js files, but
* package.json is empty or missing yet and therefore pragma cannot
* be determined based on pkg.dependencies / pkg.devDependencies
*/
lustoykov marked this conversation as resolved.
Show resolved Hide resolved
function maybeCreateFallbackPragma(asset) {
for (const dep in JSX_PRAGMA) {
if (asset.contents.match(createJSXRegexFor(dep))) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should create these regexes ahead of time rather than recreating them for every file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1 thanks for the hint, will fix tomorrow morning

return JSX_PRAGMA[dep];
}
}
}

/**
* Generates a babel config for JSX. Attempts to detect react or react-like libraries
* and changes the pragma accordingly.
Expand All @@ -37,6 +57,10 @@ async function getJSXConfig(asset, isSourceModule) {
}
}

if (!pragma) {
pragma = maybeCreateFallbackPragma(asset);
}

if (pragma || JSX_EXTENSIONS[path.extname(asset.name)]) {
return {
internal: true,
Expand Down