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.

Sign up for GitHub

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

Jump to bottom

Fix mjs handling for graphql #5257

Closed
wants to merge 1 commit into from
Closed

Fix mjs handling for graphql #5257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions fixtures/browser/issue-5234-direct-mjs-package/__snapshots__/index.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`issue #5234 (mjs files are imported as static files) correctly bundles files in development 1`] = `
Object {
"graphql": "undefined",
"parse": "function",
"test": "/static/media/Test.4478d87c.mjs",
}
`;
29 changes: 29 additions & 0 deletions fixtures/browser/issue-5234-direct-mjs-package/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
const { bootstrap, startDevelopmentServer } = require('../../utils');
const puppeteer = require('puppeteer');

beforeEach(async () => {
await bootstrap({ directory: global.testDirectory, template: __dirname });
global.appPort = await startDevelopmentServer({
directory: global.testDirectory,
});
await new Promise(resolve => setTimeout(resolve, 3000));
});

// https://github.com/facebook/create-react-app/issues/5234
describe('issue #5234 (mjs files are imported as static files)', () => {
it('correctly bundles files in development', async () => {
const browser = await puppeteer.launch({ headless: true });
Copy link
Contributor

Choose a reason for hiding this comment

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

Puppeteer! Nice.

try {
const page = await browser.newPage();
console.log(`http://localhost:${global.appPort}/`);
await page.goto(`http://localhost:${global.appPort}/`);
await page.waitForSelector('.App-Ready');
const output = await page.evaluate(() => {
return document.testData;
});
expect(output).toMatchSnapshot();
} finally {
browser.close();
}
});
});
7 changes: 7 additions & 0 deletions fixtures/browser/issue-5234-direct-mjs-package/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"dependencies": {
"react": "^16.5.2",
"react-dom": "^16.5.2",
"graphql": "14.0.2"
}
}
9 changes: 9 additions & 0 deletions fixtures/browser/issue-5234-direct-mjs-package/public/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!DOCTYPE html>
<html lang="en">
<head>
<title>React App</title>
</head>
<body>
<div id="root"></div>
</body>
</html>
6 changes: 6 additions & 0 deletions fixtures/browser/issue-5234-direct-mjs-package/src/Test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export function foo() {
console.log('fooey');
}
export default function bar() {
console.log('barrio');
}
9 changes: 9 additions & 0 deletions fixtures/browser/issue-5234-direct-mjs-package/src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!DOCTYPE html>
<html lang="en">
<head>
<title>React App</title>
</head>
<body>
<div id="root"></div>
</body>
</html>
17 changes: 17 additions & 0 deletions fixtures/browser/issue-5234-direct-mjs-package/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import React from 'react';
import ReactDOM from 'react-dom';
import graphql, { parse } from 'graphql';
import test from './Test.mjs';

class App extends React.Component {
state = { ready: false };
async componentDidMount() {
document.testData = { graphql: typeof graphql, parse: typeof parse, test };
this.setState({ ready: true });
}
render() {
return this.state.ready ? <div className="App-Ready" /> : null;
}
}

ReactDOM.render(<App />, document.getElementById('root'));
6 changes: 6 additions & 0 deletions fixtures/browser/jest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module.exports = {
testEnvironment: 'node',
testMatch: ['**/*.test.js'],
testPathIgnorePatterns: ['/src/', 'node_modules'],
setupTestFrameworkScriptFile: './setupSmokeTests.js',
};
10 changes: 10 additions & 0 deletions fixtures/browser/setupSmokeTests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
const fs = require('fs-extra');
const tempy = require('tempy');

beforeEach(() => {
global.testDirectory = tempy.directory();
jest.setTimeout(1000 * 60 * 5);
});
afterEach(() => {
fs.removeSync(global.testDirectory);
});
19 changes: 19 additions & 0 deletions fixtures/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,24 @@ async function getOutputDevelopment({ directory, env = {} }) {
}
}

async function startDevelopmentServer({ directory, env = {} }) {
const port = await getPort();
execa('./node_modules/.bin/react-scripts', ['start'], {
cwd: directory,
env: Object.assign(
{},
{
BROWSER: 'none',
PORT: port,
CI: 'false',
FORCE_COLOR: '0',
},
env
),
});
return port;
}

async function getOutputProduction({ directory, env = {} }) {
try {
const { stdout, stderr } = await execa(
Expand Down Expand Up @@ -141,5 +159,6 @@ module.exports = {
isSuccessfulProduction,
isSuccessfulTest,
getOutputDevelopment,
startDevelopmentServer,
getOutputProduction,
};
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"meow": "^5.0.0",
"multimatch": "^2.1.0",
"prettier": "1.14.3",
"puppeteer": "^1.8.0",
"strip-ansi": "^4.0.0",
"svg-term-cli": "^2.1.1",
"tempy": "^0.2.1"
Expand Down
16 changes: 14 additions & 2 deletions packages/react-scripts/config/webpack.config.dev.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,6 @@ module.exports = {
// the use of this extension, so we need to tell webpack to fall back
// to auto mode (ES Module interop, allows ESM to import CommonJS).
test: /\.mjs$/,
include: /node_modules/,
type: 'javascript/auto',
},
{
Expand Down Expand Up @@ -271,10 +270,23 @@ module.exports = {
cacheCompression: false,
},
},
// Make sure `mjs` files get processed as files, not JS. We don't
// support `mjs`, so we deliver a file for now.
{
test: /\.mjs$/,
include: paths.appSrc,
loader: require.resolve('file-loader'),
options: {
name: 'static/media/[name].[hash:8].[ext]',
},
},
// Process any JS outside of the app with Babel.
// Unlike the application JS, we only compile the standard ES features.
// `mjs` needs to be included here because some libraries forcibly
// import files with the `mjs` extension, or have set their `browser`
// or `module` field to a `mjs` file.
{
test: /\.js$/,
test: /\.m?js$/,
exclude: /@babel(?:\/|\\{1,2})runtime/,
loader: require.resolve('babel-loader'),
options: {
Expand Down
17 changes: 15 additions & 2 deletions packages/react-scripts/config/webpack.config.prod.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,6 @@ module.exports = {
// the use of this extension, so we need to tell webpack to fall back
// to auto mode (ES Module interop, allows ESM to import CommonJS).
test: /\.mjs$/,
include: /node_modules/,
type: 'javascript/auto',
},
{
Expand All @@ -284,6 +283,7 @@ module.exports = {
oneOf: [
// "url" loader works just like "file" loader but it also embeds
// assets smaller than specified size as data URLs to avoid requests.
// A missing `test` is equivalent to a match.
{
test: [/\.bmp$/, /\.gif$/, /\.jpe?g$/, /\.png$/],
loader: require.resolve('url-loader'),
Expand Down Expand Up @@ -337,10 +337,23 @@ module.exports = {
compact: true,
},
},
// Make sure `mjs` files get processed as files, not JS. We don't
// support `mjs`, so we deliver a file for now.
{
test: /\.mjs$/,
include: paths.appSrc,
loader: require.resolve('file-loader'),
options: {
name: 'static/media/[name].[hash:8].[ext]',
},
},
// Process any JS outside of the app with Babel.
// Unlike the application JS, we only compile the standard ES features.
// `mjs` needs to be included here because some libraries forcibly
// import files with the `mjs` extension, or have set their `browser`
// or `module` field to a `mjs` file.
{
test: /\.js$/,
test: /\.m?js$/,
exclude: /@babel(?:\/|\\{1,2})runtime/,
loader: require.resolve('babel-loader'),
options: {
Expand Down
3 changes: 3 additions & 0 deletions tasks/e2e-behavior.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ git clean -df
# Now that we have published them, run all tests as if they were released.
# ******************************************************************************

# Browser tests
CI=true ./node_modules/.bin/jest --config fixtures/browser/jest.config.js

# Smoke tests
CI=true ./node_modules/.bin/jest --config fixtures/smoke/jest.config.js

Expand Down