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

Run typescript sources through full babel transpiling pipeline #3369

Merged
merged 3 commits into from
Jan 25, 2018
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,26 @@ describe(`gatsby-plugin-typescript`, () => {
})

it(`modifies webpack config`, () => {
const babelConfig = { "plugins":[``] }
const config = {
loader: jest.fn(),
}

modifyWebpackConfig({ config }, { compilerOptions: {} })
modifyWebpackConfig({ config, babelConfig }, { compilerOptions: {} })

expect(config.loader).toHaveBeenCalledTimes(1)
const lastCall = config.loader.mock.calls.pop()
expect(lastCall).toMatchSnapshot()
})

it(`passes the configuration to the ts-loader plugin`, () => {
const babelConfig = { "plugins":[``] }
const config = {
loader: jest.fn(),
}
const options = { compilerOptions: { foo: `bar` }, transpileOnly: false }

modifyWebpackConfig({ config }, options)
modifyWebpackConfig({ config, babelConfig }, options)

const expectedOptions = {
compilerOptions: {
Expand All @@ -55,10 +57,11 @@ describe(`gatsby-plugin-typescript`, () => {
})

it(`uses default configuration for the ts-loader plugin when no config is provided`, () => {
const babelConfig = { "plugins":[``] }
const config = {
loader: jest.fn(),
}
modifyWebpackConfig({ config }, { compilerOptions: {} })
modifyWebpackConfig({ config, babelConfig }, { compilerOptions: {} })

const expectedOptions = {
compilerOptions: {
Expand Down
11 changes: 2 additions & 9 deletions packages/gatsby-plugin-typescript/src/gatsby-node.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
const { transpileModule } = require(`typescript`)
const path = require(`path`)

const test = /\.tsx?$/
const compilerDefaults = {
Expand All @@ -11,7 +10,7 @@ const compilerDefaults = {
module.exports.resolvableExtensions = () => [`.ts`, `.tsx`]

module.exports.modifyWebpackConfig = (
{ config },
{ config, babelConfig },
{ compilerOptions, transpileOnly = true }
) => {
// CommonJS to keep Webpack happy.
Expand All @@ -23,16 +22,10 @@ module.exports.modifyWebpackConfig = (
// error (i.e., not build) at something or other.
const opts = { compilerOptions: copts, transpileOnly }

// Load gatsby babel plugin to extract graphql query
const extractQueryPlugin = path.resolve(
__dirname,
`../gatsby/dist/utils/babel-plugin-extract-graphql.js`
)

config.loader(`typescript`, {
test,
loaders: [
`babel?${JSON.stringify({ plugins: [extractQueryPlugin] })}`,
`babel?${JSON.stringify(babelConfig)}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

rather then passing through the babel config can you stick the ts loader in front of the existing js loader in the chain. You can also "override" the js loader and pass a function the gets the current one and alters it.

its not clean, but this will be much easier to do neatly in v2, and i don't think we want to add more surface area if we can avoid it, since its much harder to deprecate later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I do that then typescript becomes a hard dependency. I don't think that makes sense for most people using gatsby.

`ts-loader?${JSON.stringify(opts)}`,
],
})
Expand Down
17 changes: 15 additions & 2 deletions packages/gatsby/src/commands/build-javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,20 @@ module.exports = async program => {
`build-javascript`
)

return new Promise(resolve => {
webpack(compilerConfig.resolve()).run(() => resolve())
return new Promise((resolve, reject) => {
webpack(compilerConfig.resolve()).run((err, stats) => {
if (err) {
reject(err)
return
}

const jsonStats = stats.toJson()
if (jsonStats.errors && jsonStats.errors.length > 0) {
reject(jsonStats.errors[0])
return
}

resolve()
})
})
}
4 changes: 2 additions & 2 deletions packages/gatsby/src/utils/webpack-modify-validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ const validationWhitelist = Joi.object({
responsiveLoader: Joi.any(),
})

export default (async function ValidateWebpackConfig(config, stage) {
export default (async function ValidateWebpackConfig(program, config, babelConfig, stage) {
// We don't care about the return as plugins just mutate the config directly.
await apiRunnerNode(`modifyWebpackConfig`, { config, stage })
await apiRunnerNode(`modifyWebpackConfig`, { program, config, babelConfig, stage })

// console.log(JSON.stringify(config, null, 4))

Expand Down
5 changes: 2 additions & 3 deletions packages/gatsby/src/utils/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,12 @@ module.exports = async (
webpackPort = 1500,
pages = []
) => {
const babelStage = suppliedStage
const directoryPath = withBasePath(directory)

// We combine develop & develop-html stages for purposes of generating the
// webpack config.
const stage = suppliedStage
const babelConfig = await genBabelConfig(program, babelStage)
const babelConfig = await genBabelConfig(program, suppliedStage)

function processEnv(stage, defaultNodeEnv) {
debug(`Building env for "${stage}"`)
Expand Down Expand Up @@ -556,7 +555,7 @@ module.exports = async (

// Use the suppliedStage again to let plugins distinguish between
// server rendering the html.js and the frontend development config.
const validatedConfig = await webpackModifyValidate(config, suppliedStage)
const validatedConfig = await webpackModifyValidate(program, config, babelConfig, suppliedStage)

return validatedConfig
}