Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

next/babel preset hard-codes directory path in output #1160

Closed
fionawhim opened this issue Feb 15, 2017 · 7 comments
Closed

next/babel preset hard-codes directory path in output #1160

fionawhim opened this issue Feb 15, 2017 · 7 comments

Comments

@fionawhim
Copy link
Contributor

The babelRuntimePath at the beginning of the babel/preset.js file results in the absolute path to the babel runtime being hard-coded into babel output. This means that e.g. server code cannot be babel-compiled in one location and run on another.

I'm running into this by babel-compiling my server code on one machine and deploying the compiled versions to a server and running them there.

@fionawhim
Copy link
Contributor Author

Poking around, I think it might work to remove all of the path up to and including the first instance of “node_modules/”:

const pathStart = 'node_modules/'
babelRuntimePath = babelRuntimePath.substring(babelRuntimePath.indexOf(pathStart) + pathStart.length)

Does that make sense as something to do?

@fionawhim
Copy link
Contributor Author

This seems to be an issue even when compiling directly on Heroku, as the build-time path and the runtime path are different.

Note that I'm using the next/babel preset globally for all my JS, both webclient and server, so that everything is consistent as far as babel features.

@fionawhim
Copy link
Contributor Author

Looks like the webpack config is doing this, too. Here’s what it looks like in Heroku logs:

2017-02-15T19:04:09.462454+00:00 app[web.1]: { Error: Cannot find module '/tmp/build_c63e484ca4c05f0e45d82a9e42201fc2/node_modules/next/dist/lib/head.js'
2017-02-15T19:04:09.462459+00:00 app[web.1]:     at Function.Module._resolveFilename (module.js:470:15)
2017-02-15T19:04:09.462460+00:00 app[web.1]:     at Function.Module._load (module.js:418:25)
2017-02-15T19:04:09.462461+00:00 app[web.1]:     at Module.require (module.js:498:17)
2017-02-15T19:04:09.462461+00:00 app[web.1]:     at require (internal/module.js:20:19)
2017-02-15T19:04:09.462462+00:00 app[web.1]:     at Object.<anonymous> (/app/.next/dist/pages/report.js:39:13)
2017-02-15T19:04:09.462463+00:00 app[web.1]:     at Module._compile (module.js:571:32)
2017-02-15T19:04:09.462463+00:00 app[web.1]:     at Object.Module._extensions..js (module.js:580:10)
2017-02-15T19:04:09.462464+00:00 app[web.1]:     at Module.load (module.js:488:32)
2017-02-15T19:04:09.462464+00:00 app[web.1]:     at tryModuleLoad (module.js:447:12)
2017-02-15T19:04:09.462465+00:00 app[web.1]:     at Function.Module._load (module.js:439:3) code: 'MODULE_NOT_FOUND' }

Note how Heroku built in a tmp directory but is running from an /app directory.

I can have a PR for this up pretty soon.

fionawhim pushed a commit to fionawhim/next.js that referenced this issue Feb 15, 2017
Without this, modules built with Babel or Webpack would have hard-coded absolute paths
all the way back to the root of the filesystem. This prevented compilation and running
on different machines or even from different directories on the same machine.

With this change, paths are hard-coded to the top-most node_madules directory found,
which should make them portable relative to the app.

Fixes vercel#1160
@bensalilijames
Copy link
Contributor

There's some more context for you in #198 and #360!

The current solution is using a Heroku buildpack: mars/heroku-nextjs

I had hoped it would be as simple as removing everything up to node_modules in the path too (see PR #361) but there are a number of reasons why that didn't work

@fionawhim
Copy link
Contributor Author

Are the concerns in #361 still valid? React is now a peerDependency. The relative importing of babel-runtime seems ok to me, but TBH that could probably get moved to a peerDependency as well.

I guess I also don't understand why the next/head aliases are in there as well, given that Next has a head.js file in its root that would be resolved by next/head without webpack/babel shenanigans.

@bensalilijames
Copy link
Contributor

There are a few remaining concerns (though like you say, React as a peer mitigates one of them!)

The next/[module-name] aliases are only required when running tests - we can guarantee that they'll always exist when using next as a module. So if node_modules will never exist when doing local development on next, the path it gets back after our exciting resolution algorithm is non-existant or weird.

I had toyed around with adding a "we are not running tests so pretty please don't use aliases on next modules" flag (or vice versa) - maybe this is something to be revisited.

babel-runtime and styled-jsx would need to also become peer dependencies (if we want to guarantee that the user is using a version of them that next supports - otherwise we would need to put our trust in the user never to install another version of it).

There's also some issues surrounding using next as a symlinked package (i.e. node_modules might not exist in the resolved absolute path).

@fionawhim
Copy link
Contributor Author

If the aliases are only needed for running Next.js’s own tests, it seems like the burden should be on the Next.js dev/test setup to get those right, rather than on every developer to manage the surprise of these hard-coded paths on their own.

Compiling JS in one place and serving it somewhere else doesn't seem like so esoteric a pattern to me that it shouldn't be supported out-of-the-box.

fionawhim pushed a commit to fionawhim/next.js that referenced this issue Feb 15, 2017
Without this, modules built with Babel or Webpack would have hard-coded absolute paths
all the way back to the root of the filesystem. This prevented compilation and running
on different machines or even from different directories on the same machine.

With this change, paths are hard-coded to the top-most node_madules directory found,
which should make them portable relative to the app.

Fixes vercel#1160
fionawhim pushed a commit to fionawhim/next.js that referenced this issue Feb 15, 2017
Without this, modules built with Babel or Webpack would have hard-coded absolute paths
all the way back to the root of the filesystem. This prevented compilation and running
on different machines or even from different directories on the same machine.

With this change, paths are hard-coded to the top-most node_madules directory found,
which should make them portable relative to the app.

Fixes vercel#1160
fionawhim pushed a commit to fionawhim/next.js that referenced this issue Feb 15, 2017
Without this, modules built with Babel or Webpack would have hard-coded absolute paths
all the way back to the root of the filesystem. This prevented compilation and running
on different machines or even from different directories on the same machine.

With this change, paths are hard-coded to the top-most node_madules directory found,
which should make them portable relative to the app.

Fixes vercel#1160
arunoda pushed a commit that referenced this issue Feb 16, 2017
Without this, modules built with Babel or Webpack would have hard-coded absolute paths
all the way back to the root of the filesystem. This prevented compilation and running
on different machines or even from different directories on the same machine.

With this change, paths are hard-coded to the top-most node_madules directory found,
which should make them portable relative to the app.

Fixes #1160
@lock lock bot locked as resolved and limited conversation to collaborators May 12, 2018
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants