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

Add support for transpile packages installed via npm #3319

Closed
wants to merge 4 commits into from
Closed

Add support for transpile packages installed via npm #3319

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a5e56f2
Add support for transpile packages installed via npm
giuseppeg Nov 22, 2017
45ba229
Add example and docs
giuseppeg Nov 26, 2017
4a4812f
Merge branch 'canary' into transpile-npm-packages
timneutkens Dec 6, 2017
a577529
Copy package.json POF
giuseppeg Dec 24, 2017
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
7 changes: 7 additions & 0 deletions server/build/loaders/emit-file-loader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ module.exports = function (content, sourceMap) {
callback(null, code, map)
}

if (query.copyPackageJson) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should/could be a separate loader

const info = query.copyPackageJson(interpolatedName)
if (info) {
this.emitFile(info.interpolatedName, info.content)
}
}

if (query.transform) {
const transformed = query.transform({ content, sourceMap, interpolatedName })
return emit(transformed.content, transformed.sourceMap)
Expand Down
40 changes: 37 additions & 3 deletions server/build/webpack.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { resolve, join, sep } from 'path'
import { resolve, join, sep, dirname } from 'path'
import { createHash } from 'crypto'
import { realpathSync, existsSync } from 'fs'
import { realpathSync, existsSync, readFileSync } from 'fs'
import webpack from 'webpack'
import glob from 'glob-promise'
import WriteFilePlugin from 'write-file-webpack-plugin'
Expand Down Expand Up @@ -203,6 +203,24 @@ export default async function createCompiler (dir, { buildId, dev = false, quiet
})
}

function getPackageJsonPath (filePath) {
Copy link

Choose a reason for hiding this comment

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

Consider using https://www.npmjs.com/package/find-root instead

function getPackageJson (filePath) {
  const pkgRoot = findRoot(filePath)
  const packageJsonPath = path.resolve(pkgRoot, 'package.json')

  return packageJsonPath
}

let filePathDirname = dirname(filePath)
if (filePathDirname === dir) {
return join(dir, 'package.json')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

need to check whether this file exists or probably return undefined in this case.

}

while (dir !== filePathDirname) {
const packageJsonPath = join(filePathDirname, 'package.json')
if (existsSync(packageJsonPath)) {
return packageJsonPath
}
filePathDirname = dirname(filePathDirname)
}
}

const packageJsonToCopy = {}
const copiedPackageJson = {}
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 sure if this is persisted across builds (when rebuilding)


const rules = (dev ? [{
test: /\.(js|jsx)(\?[^?]*)?$/,
loader: 'hot-self-accept-loader',
Expand All @@ -224,6 +242,12 @@ export default async function createCompiler (dir, { buildId, dev = false, quiet
include: [dir, nextPagesDir],
exclude (str) {
if (shouldTranspileModule(str)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This works for sure.
For the SSR version, we may need to rewrite the require('my-module') pointing to this emitted file.

And if that requires a ES6+ module, how we are going to handle it?

Copy link
Contributor Author

@giuseppeg giuseppeg Nov 23, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This works for SSR since node will look for .next/dist/node_modules first and pick up the transpiled version in it.

And if that requires a ES6+ module, how we are going to handle it?

During my testing my-modules was depending on another es6 module (in the same package) and both were transpiled and emitted to .next/dist/node_modules/my-modules.

Later today or tomorrow (when I have time basically) I will add those to the PR together with the changes to the readme.

As I said this solution still doesn't work with linked packages though - I will try to tackle that at the end and see if it is easy/doable to fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

@giuseppeg

This works for SSR since node will look for .next/dist/node_modules first and pick up the transpiled version in it.

That's pretty interesting. Nice work.

Looking forward to take this in.

Copy link
Contributor Author

@giuseppeg giuseppeg Nov 26, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@arunoda edit: in order for this to work we need to copy the package.json over too. Do you have any idea how to do so? Or would you rather rewrite the imports/requires?

if (!packageJsonToCopy[str]) {
const packageJsonPath = getPackageJsonPath(str)
if (packageJsonPath) {
packageJsonToCopy[str.replace(dir, '')] = packageJsonPath
}
}
return false
}

Expand All @@ -243,12 +267,22 @@ export default async function createCompiler (dir, { buildId, dev = false, quiet
}

const filePath = file.slice(0, -(from.length)) + to

if (existsSync(filePath)) {
throw new Error(`Both ${from} and ${to} file found. Please make sure you only have one of both.`)
}
}
},
copyPackageJson (interpolatedName) {
const packageJsonPath = packageJsonToCopy[interpolatedName.replace(/^dist/, '')]

if (packageJsonPath && !copiedPackageJson[packageJsonPath]) {
copiedPackageJson[packageJsonPath] = true
return {
interpolatedName: join('dist', packageJsonPath.replace(dir, '')),
content: readFileSync(packageJsonPath, 'utf-8')
}
}
},
// By default, our babel config does not transpile ES2015 module syntax because
// webpack knows how to handle them. (That's how it can do tree-shaking)
// But Node.js doesn't know how to handle them. So, we have to transpile them here.
Expand Down