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

Resolve .babelrc files in a monorepo. #4132

Merged
merged 8 commits into from Feb 25, 2020
Merged

Resolve .babelrc files in a monorepo. #4132

merged 8 commits into from Feb 25, 2020

Conversation

astegmaier
Copy link
Contributor

@astegmaier astegmaier commented Feb 14, 2020

↪️ Pull Request

This PR contains a possible fix to #4120 (might also address one of the causes of #3917). The goal is to allow parcel2 to pick up configuration specified in .babelrc files in a monorepo.

I wanted to make sure that the problem I experienced was considered holistically (taking into account all the likely ways that babel users are used to specifying configuration). As it turned out, that was kind of complicated, so I made a detailed explanation here:

https://astegmaier.github.io/parcel2-monorepo-babel-bug/parcel-goals

I'm putting this out there for early feedback. Specifically:

  1. Goals - does my scenario list make sense?
  2. Performance - in order to implement the desired behavior, I needed to determine the sub-project's directory in the process of processing each compiled file, so I can pass it to the babelrcRoots property of the babel api. The code I wrote makes calls to the file system each time a file's config is desired. I wonder if this information might be cached (maybe it already is, and I didn't realize how to access it)?
  3. Unit Tests - I would love some pointers about how/where to do this, patterns to follow, and how much of the scenario list you think is important to unit test. Edit (2/24): This is done.
  4. Documentation - There's a lot of nuance here, and it would be great if the final parcel2 release came with clear, complete documentation about how to get this working. Is there a place where I could put a draft (to be incorporated into the final site down the road)?

💻 Examples

The main goal is to make sure that Parcel2 will pick up .babelrc files in sub-projects of a monorepo. For example, if you have a project like this...

├── package.json // {"workspaces": ["app1"]}
├── app1
│   ├── .babelrc // Provides plugins/presets to allow index.js to be compiled.
│   ├── index.js // uses some advanced JS feature that Babel won't compile by default
│   └── package.json
└── yarn.lock

...things will work.

🚨 Test instructions

See this test repo for an environment where you can see the issue and test out the changes.

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

Closes #4120

@DeMoorJasper
Copy link
Member

You goals make total sense to me, I agree that sub-projects babelrc files should be respected like the babel-cli does.

Copy link
Member

@DeMoorJasper DeMoorJasper left a comment

Choose a reason for hiding this comment

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

Looks good to me, an integration test for this would be awesome

@astegmaier
Copy link
Contributor Author

I added a unit test for the most complicated scenario (i.e. the one where you have to merge a .babelrc from a sub-project with babel.config.json from the root). Although this is not 100% comprehensive, it seems likely that anything that would break this scenario would also break the less-complicated scenarios (e.g. you only have .babelrc in the sub-project without any babel.config.json in the root), and it was valuable to keep the integration tests from ballooning.

@DeMoorJasper - let me know if you'd like to see any changes before the merge, or if there's any place where I can help with documentation.

@DeMoorJasper DeMoorJasper merged commit bb5ad3d into parcel-bundler:v2 Feb 25, 2020
if (packageJSONPath) {
let packageRoot = path.dirname(packageJSONPath);
if (packageRoot && packageRoot !== options.projectRoot) {
babelrcRoots.push(packageRoot);
Copy link
Member

Choose a reason for hiding this comment

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

We probably also need to track the package.json paths we searched as part of the config so it will be invalidated in the cache properly. Maybe a glob? cc. @padmaia

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yeah, I guess that never got set. Easy to add though, it's just a call to config.setWatchGlob().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I fully understand how the cache invalidation works - could you elaborate on what you expect to happen (but isn't already), and how I might be able to test it?

In case it's relevant, one thing to note is that the sub-project babel config might be specified in a .babelrc/babelrc.json file in the sub-package directory, or as part of the babel property of the package.json file. Presumably, we'd want to make sure that the bundle will get regenerated if any of these change (and do whatever is necessary to keep the cache clean). Given this, are there any other tweaks you think might be necessary?

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 this pull request may close these issues.

Parcel2 ignores .babelrc files in monorepos.
4 participants