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

babel-register compiles .js files; should only handle .babel.js #39

Closed
cspotcode opened this issue Jun 21, 2016 · 12 comments · Fixed by #58
Closed

babel-register compiles .js files; should only handle .babel.js #39

cspotcode opened this issue Jun 21, 2016 · 12 comments · Fixed by #58

Comments

@cspotcode
Copy link
Contributor

Babel frequently has bugs (at least, I've hit many) so there's no reason to have babel compiling .js files when it's not necessary.

You can use babel-register's only option to avoid transpiling files unless they actually end in .babel.js
https://github.com/js-cli/js-interpret/blob/master/index.js#L2-L12

'.babel.js': [
    {
      module: 'babel-register',
      register: function (module) {
        module({
          // register on .js extension due to https://github.com/joyent/node/blob/v0.12.0/lib/module.js#L353
          // which only captures the final extension (.babel.js -> .js)
          extensions: '.js',
          only: /\.babel\.js$/
        });
      }
    },
    // ... do the same for other entries in this array
@phated
Copy link
Member

phated commented Jun 21, 2016

That's super cool but it didn't exist when we implemented this. This addition should be fine as older versions should ignore that extra property (people might still encounter a problem with older versions).

@graingert
Copy link

@phated this will break my codebase because we use a mix of ".js" and ".es" files.

Maybe interpret should read settings from ".interpret.json" or "package.json#interpret"

@cspotcode
Copy link
Contributor Author

@graingert: How will this break your code, since js-interpret doesn't do anything for .es files?

@phated
Copy link
Member

phated commented Jun 28, 2016

@graingert We won't be adding anything like an .interpret.json because this is just a config file as a module :/

@graingert
Copy link

I'm using both .js and .es files I expect them both to be transpiled
On 28 Jun 2016 4:34 am, "Blaine Bublitz" notifications@github.com wrote:

@graingert https://github.com/graingert We won't be adding anything
like an .interpret.json because this is just a config file as a module :/


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#39 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAZQTBG1LPj02Hvl04ia3ml4hZrONJAVks5qQJZOgaJpZM4I7NsB
.

cspotcode added a commit to cspotcode/js-interpret that referenced this issue Jun 28, 2016
@cspotcode
Copy link
Contributor Author

Right, but js-interpret specifically does not transpile .es files, so your codebase should already be broken unless I'm mistaken. Like @phated said, this module is just a config file.

@graingert
Copy link

graingert commented Jun 28, 2016

yeah I get that, but once the first module loads all the ".js" files are also transpiled. Which is a step forward.

@cspotcode
Copy link
Contributor Author

...but .es files aren't transpiled, and you said you need .es files
to be transpiled.

I think your use case it in conflict with js-interpret's intended behavior.
js-interpret loads a transpiler / module loader if a given file extension
indicates it is required. Per js-interpret's configuration, only
.babel.js files should be transpiled. .js files, according to line
50
should
not be transpiled, so PR #41 makes the behavior match that intent.

On Tue, Jun 28, 2016 at 11:49 AM, Thomas Grainger notifications@github.com
wrote:

yeah I get that, but once this loads all the ".js" files are also
transpiled.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#39 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAW-uFYXuZd9uaqiEif5oAGaChnNfCrCks5qQUKkgaJpZM4I7NsB
.

@graingert
Copy link

graingert commented Jun 28, 2016

Yeah I want ".es" and ".js". Without this change, interpret is half-way there. And I currently depend on that feature.

@tkellen
Copy link

tkellen commented Jun 29, 2016

@graingert This module is primarily used to support writing cli tool configuration files in any format transparently. What is the use case where you have, for example, a gulpfile with two different extensions?

@graingert
Copy link

I need to require my webpack config in my backend codebase (enhanced require etc). I have a mix of legacy ES5 code and ES2016 code that need to be linted differently. JSCS does not support distinguishing of files like .babel.js and .js, they both have the same extension.

So I use a mix of .js and .es files, and I configure two JSCS and eslint configs, one that points at .js the other at .es.

@phated
Copy link
Member

phated commented Jun 29, 2016

@graingert we aren't going to support the .es extensions because it was thoroughly discussed in the gulp issue gulpjs/gulp#830

phated pushed a commit that referenced this issue Oct 16, 2019
…ward-optional mode (fixes #39, #41, #54) (#58)

* babel must now ignore files not ending by babel.js (inspired from @cspotcode branch, PR #41 extending ignoring to everywhere babel register is called)
* @babel/register now uses upward-optional mode
phated pushed a commit that referenced this issue Oct 16, 2019
…ward-optional mode (fixes #39, #41, #54) (#58)

* babel must now ignore files not ending by babel.js (inspired from @cspotcode branch, PR #41 extending ignoring to everywhere babel register is called)
* @babel/register now uses upward-optional mode
HRKings pushed a commit to HRKings/interpret that referenced this issue Jan 15, 2022
…ward-optional mode (fixes gulpjs#39, gulpjs#41, gulpjs#54) (gulpjs#58)

* babel must now ignore files not ending by babel.js (inspired from @cspotcode branch, PR gulpjs#41 extending ignoring to everywhere babel register is called)
* @babel/register now uses upward-optional mode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants