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 all 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 @@ -1390,6 +1390,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 @@ -1400,6 +1422,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 @@ -1410,14 +1454,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
32 changes: 32 additions & 0 deletions packages/core/parcel-bundler/src/transforms/babel/jsx.js
Expand Up @@ -12,6 +12,34 @@ 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
const cacheJsxRegexFor = {};
Copy link
Contributor Author

@lustoykov lustoykov Jan 17, 2019

Choose a reason for hiding this comment

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

@devongovett, hope that goes in direction of your proposal. If I understood correctly, the intention behind it was to avoid instantiating the same regex multiple times. Regexes are now lazily created in case needed (in other words: in case maybeCreateFallbackPragma gets called) and then cached (similarly to ahead of time creation).

function maybeCreateFallbackPragma(asset) {
for (const dep in JSX_PRAGMA) {
let regex = cacheJsxRegexFor[dep];

if (!regex) {
regex = createJSXRegexFor(dep);
cacheJsxRegexFor[dep] = regex;
}

if (asset.contents.match(regex)) {
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 +65,10 @@ async function getJSXConfig(asset, isSourceModule) {
}
}

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

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