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

Handle cycles of plugins compiling themselves and .babelrc.js files loading themselves #5586

Merged
merged 2 commits into from Aug 29, 2017

Conversation

@loganfsmyth
Copy link
Member

loganfsmyth commented Apr 5, 2017

Q A
Patch: Bug Fix? Y
Major: Breaking Change? N
Minor: New Feature? N
Deprecations?
Spec Compliancy?
Tests Added/Pass?
Fixed Tickets
License MIT
Doc PR
Dependency Changes
  • Silently skips require-hook cycles of compiling files used inside a .babelrc.js.
  • Throw a helpful error when a plugin tries to compile itself from a require hook.
  • Add a bunch of debug() calls to help debug things.
@mention-bot

This comment has been minimized.

Copy link

mention-bot commented Apr 5, 2017

@loganfsmyth, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jamestalmage, @existentialism and @kaicataldo to be potential reviewers.

@loganfsmyth loganfsmyth force-pushed the loganfsmyth:config-dependency-cycles branch from c69867d to 70e9905 Apr 5, 2017
@codecov

This comment has been minimized.

Copy link

codecov bot commented Apr 5, 2017

Codecov Report

Merging #5586 into 7.0 will decrease coverage by 0.03%.
The diff coverage is 68.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##              7.0    #5586      +/-   ##
==========================================
- Coverage   84.32%   84.29%   -0.04%     
==========================================
  Files         285      285              
  Lines        9761     9791      +30     
  Branches     2737     2739       +2     
==========================================
+ Hits         8231     8253      +22     
- Misses       1014     1020       +6     
- Partials      516      518       +2
Impacted Files Coverage Δ
...ckages/babel-core/src/config/build-config-chain.js 75.58% <100%> (+1.5%) ⬆️
...ges/babel-core/src/config/loading/files/plugins.js 68.67% <60%> (-2.34%) ⬇️
...bel-core/src/config/loading/files/configuration.js 85.71% <63.63%> (-3.04%) ⬇️
...bel-plugin-transform-es2015-classes/src/vanilla.js 90.17% <0%> (+0.42%) ⬆️
packages/babel-traverse/src/path/context.js 85.34% <0%> (+0.86%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6af8e64...83e664c. Read the comment docs.

@loganfsmyth loganfsmyth force-pushed the loganfsmyth:config-dependency-cycles branch 3 times, most recently from b97548e to c6818fe Apr 10, 2017
options: {
// Return a pattern that matches all files. Any file depending on this config will be skipped when
// it is compiled, if the file is loaded from inside the '.babelrc.js' file.
ignore: [ /^/ ],

This comment has been minimized.

Copy link
@loganfsmyth

loganfsmyth Apr 14, 2017

Author Member

I go back and forth if we should include this. The alternative is that we just return an empty object, so files that are referenced from inside .babelrc.js will act like the .babelrc file was empty, which would allow them to still be processed by presets passed directly to babel-register programmatically. That would allow some transformation, but might just be more confusing because then the files could end up partially compiled.

@loganfsmyth loganfsmyth force-pushed the loganfsmyth:config-dependency-cycles branch 2 times, most recently from b619769 to 83e664c Apr 17, 2017
@loganfsmyth loganfsmyth force-pushed the loganfsmyth:config-dependency-cycles branch from 83e664c to 2846e06 Aug 29, 2017
@babel-bot

This comment has been minimized.

Copy link
Collaborator

babel-bot commented Aug 29, 2017

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/4736/

1 similar comment
@babel-bot

This comment has been minimized.

Copy link
Collaborator

babel-bot commented Aug 29, 2017

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/4736/

@loganfsmyth loganfsmyth merged commit d79a792 into babel:7.0 Aug 29, 2017
4 checks passed
4 checks passed
babel/repl REPL preview is available
Details
ci/circleci Your tests passed on CircleCI!
Details
codecov/project 86.04% (target 80%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@loganfsmyth loganfsmyth deleted the loganfsmyth:config-dependency-cycles branch Aug 29, 2017
@lock lock bot added the outdated label Oct 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.