Skip to content

Return a list of files that were read from loadPartialConfig #11907

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

Jump to bottom
Merged
nicolo-ribaudo merged 3 commits into from
Oct 9, 2020
Merged

Return a list of files that were read from loadPartialConfig #11907

nicolo-ribaudo merged 3 commits into from
Oct 9, 2020

Conversation

@devongovett
Copy link
Contributor

@devongovett devongovett commented Aug 3, 2020 •
edited by JLHwung
Loading

This adds a files list to the return value of babelCore.loadPartialConfig which lists all files that were read in the process of loading the config. Some of these are currently returned in the babelrc, babelignore, and configFile fields, but other files such as those that were extended from another config are not returned, making it difficult for other tools that consume babel (e.g. Parcel, webpack) to properly watch these extended configs and invalidate caches. The files key should solve this as it includes all files that influenced the config, not just the top-level ones.

In addition, I've added a showIgnoredFiles option to loadPartialConfig which always returns a PartialConfig result even when the file is ignored, rather than returning null. This allows tools to properly invalidate caches when whatever config file that marked the compiled file as ignored changes. An isIgnored boolean is returned as part of the PartialConfig result to indicate that the file is ignored. Given that changing the null result to return a value would be a breaking change, this is behind an option. Open to better name suggestions though! 😉

This is really important for us in Parcel so that we can get proper caching working with Babel. At the moment there's really no good way to do that without support from Babel itself, or reinventing all of its config loading. This should also help other tools like babel-loader - found a todo comment for it. 😄

Totally open to alternative suggestions. Really made this to get the conversation started.

Q                       A
Fixed Issues?
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature? Yes
Tests Added + Pass? Yes
Documentation PR Link babel/website#2333
Any Dependency Changes?
License MIT

@babel-bot
Copy link
Collaborator

babel-bot commented Aug 3, 2020 •
edited
Loading

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

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 3, 2020 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 0b92084:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration

@existentialism existentialism added PR: New Feature 🚀 A type of pull request used for our changelog categories pkg: core labels Aug 4, 2020
JLHwung
JLHwung reviewed Aug 4, 2020
View reviewed changes
nicolo-ribaudo
nicolo-ribaudo reviewed Aug 4, 2020
View reviewed changes
@devongovett
Copy link
Contributor Author

devongovett commented Aug 25, 2020

Anything I can do to move this forward? 😄

JLHwung
JLHwung approved these changes Sep 10, 2020
View reviewed changes
Copy link
Contributor

@JLHwung JLHwung left a comment

Choose a reason for hiding this comment

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

Look good to me. If we get enough approvals, will need to update https://babeljs.io/docs/en/babel-core#loadpartialconfig

@devongovett
Copy link
Contributor Author

devongovett commented Sep 11, 2020

I'm happy to send a PR for that!

@JLHwung JLHwung added this to the v7.12.0 milestone Sep 11, 2020
devongovett added a commit to devongovett/website that referenced this pull request Sep 20, 2020
@devongovett
Add docs for loadPartialConfig file handling changes
c9a01ac 
babel/babel#11907
@devongovett devongovett mentioned this pull request Sep 20, 2020
Merged
@devongovett
Copy link
Contributor Author

devongovett commented Sep 20, 2020

babel/website#2333

@JLHwung JLHwung added PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release and removed PR: Needs Docs labels Sep 24, 2020
@nicolo-ribaudo nicolo-ribaudo mentioned this pull request Oct 7, 2020
Closed
4 tasks
@nicolo-ribaudo nicolo-ribaudo changed the base branch from main to feat-7.12.0/config-loading-updates October 9, 2020 14:31
@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Oct 9, 2020

I'm merging this to feat-7.12.0/config-loading-updates, so that I can rebase #11689.

@nicolo-ribaudo nicolo-ribaudo merged commit 05b857a into babel:feat-7.12.0/config-loading-updates Oct 9, 2020
@nicolo-ribaudo nicolo-ribaudo mentioned this pull request Oct 9, 2020
Merged
3 tasks
@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Oct 9, 2020 •
edited
Loading

@devongovett @JLHwung @existentialism @kaicataldo When I asked for fileHandling to be a string so that it can allow three values (transpile, ignored and unsupported), I was thinking that I could use it for 9114ffc (#12151): my idea was to load the config for a file, check if it's "ignored" or "unsupported", and act accordingly.

However, while implementing it I realized that it's not possible because loadPartialConfig does not resolve presets, and thus it doesn't have info about the supported extensions.

I found another way of detecting ignored vs unsupported: if loadPartialConfig returns null it's ignored; otherwise if transformFile returns null it's unsupported.

For this reason, I propose to change back the design of this PR to isIgnored: boolean rather than fileHandling: string. I can work on it this weekend if you agree, and I'm sorry for having asked @devongovett to do some unnecessary work.

@JLHwung
Copy link
Contributor

JLHwung commented Oct 10, 2020

@nicolo-ribaudo I don't have strong preference here. But I believe

I found another way of detecting ignored vs unsupported: if loadPartialConfig returns null it's ignored; otherwise if transformFile returns null it's unsupported.

should be documented somewhere. If fileHandling: StatusEnum is more friendly to API users, we can go with current design.

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Oct 14, 2020

Sure, I'll document it alongside with the extensions option docs!

nicolo-ribaudo pushed a commit that referenced this pull request Oct 14, 2020
@devongovett @nicolo-ribaudo
Return a list of files that were read from loadPartialConfig (#11907)
ce7b170
@MwumLi MwumLi mentioned this pull request Oct 16, 2020
Closed
@devongovett devongovett mentioned this pull request Oct 19, 2020
Merged
JLHwung pushed a commit to babel/website that referenced this pull request Oct 21, 2020
@devongovett
Add docs for loadPartialConfig file handling changes (#2333)
9117aa5 
* Add docs for loadPartialConfig file handling changes

babel/babel#11907

* Update core.md
@JLHwung JLHwung mentioned this pull request Dec 9, 2020
Merged
11 tasks
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jan 14, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@nicolo-ribaudo nicolo-ribaudo nicolo-ribaudo left review comments

@existentialism existentialism existentialism approved these changes

@JLHwung JLHwung JLHwung approved these changes

@kaicataldo kaicataldo kaicataldo approved these changes

Assignees

No one assigned

Labels

outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: core PR: New Feature 🚀 A type of pull request used for our changelog categories PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release

Projects

None yet

Milestone

v7.12.0

Development

Successfully merging this pull request may close these issues.

6 participants

@devongovett @babel-bot @nicolo-ribaudo @JLHwung @existentialism @kaicataldo