Allow plugins to indicate dependencies on random files #11741

vedantroy · 2020-06-23T07:32:38Z

Q	A
Fixed Issues?	Fixes #8497
Patch: Bug Fix?	No
Major: Breaking Change?	No
Minor: New Feature?	Yes
Tests Added + Pass?	No (existing tests pass, but need help on adding new tests)
Documentation PR Link
Any Dependency Changes?	No
License	MIT

I took a shot at fixing #8497. ~~The fix seems surprisingly simple, so I might be missing something obvious.~~ (was missing something obvious)

This PR adds a addExternalDependency function to Babel's plugin API. Plugins can call this function with 2 values:

the path of the external dependency
the path of the dependent file (usually the current file the plugin is processing).

Calling addExternalDependency establishes a 2 way-mapping. Specifically, @babel/core exports 2 methods:

getExternalDependencies - This method returns a map of external dependency names to their dependent files
getDependencies - This method returns a map of source file names to the non-source files (external dependencies) that depend on them.

This PR integrates the above information with @babel/cli's --watch flag. Whenever a file is compiled in watch mode, @babel/cli will check the above information and watch any new external dependencies.

This is a draft PR because there's a few things missing:

~~tests (I'm not sure how to test this, given that it's not a normal code transformation)~~
- there are cli tests, but writing watch tests seems much more complicated. I will look more into this. The ideal test would be one that loads a plugin that relies on a non-source file, and then transforming that non-source file during the test to ensure the source file is also transformed. This seems complicated, but also the most comprehensive option.
I need to make the code work with flow better
should I rename dependencies/externalDependencies to sourceFile/nonSourceFile

This PR could work in conjunction with a PR to babel-loader. Specifically, babel-loader could use getExternalDependencies to call webpack's addDependency.

codesandbox-ci · 2020-06-23T17:03:32Z

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 8dfab48:

Sandbox	Source
epic-fog-7pxwq	Configuration
misty-cache-wjsr4	Configuration

babel-bot · 2020-06-23T17:04:16Z

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

packages/babel-core/src/config/full.js

packages/babel-core/src/index.js

vedantroy · 2020-06-23T22:40:53Z

@JLHwung I'm integrating this with @babel/cli, so the watch method will recompile a file when an external dependency that is associated with it has changed.

Do you know why disableGlobbing is set to true, here, but not set to true here.

JLHwung · 2020-06-23T22:46:38Z

@vedantroy

Do you know why disableGlobbing is set to true, here, but not set to true here.

Good catch, I believe disableGlobbing should be set for the latter, too.

vedantroy · 2020-06-24T00:42:55Z

@JLHwung, another question!

I notice that the code for determining whether to recompile a file is different in file.js and dir.js. Specifically, file.js has a check that dir.js does not have. (dir.js does not call !util.isCompilableExtension && !filenames.includes(filename)). Which version is right / or is there a reason that file.js has additional checks?

Context: As I'm performing the integration of @babel/cli's watch mode with the external dependency tracking, I'm consolidating some of the code in dir.js and file.js in utils.js.

Edit: It seems like there is a bug in file.js because the check !filenames.includes(filename) would exclude some valid files. For example, if the directory foo/ is in filenames, then foo/bar.js would be excluded from watch mode because foo/bar.js is not part of ["foo/"]. I think right now this bug does not occur in practice because the first check (!util.isCompilableExtension) returns false, so the if statement never triggers.

Another question is: What does "file.js" do to handle the scenario where multiple files are changed in rapid succession? It seems like chokidar will call the compilation method multiple times, but, there doesn't seem to be any enforcement of ordering. E.g, the callbacks are not stored in a queue, so theoretically an older callback could overwrite a newer callback.

JLHwung · 2020-06-24T18:35:16Z

Which version is right / or is there a reason that file.js has additional checks?

I think it should also be applied for dir.js. The changes you mentioned before was made by me one years ago. I have no idea if I had special reasons other than that I simply overlooked them 😄.

I'm consolidating some of the code in dir.js and file.js in utils.js

Can you address them in a separate PR? So we can fast track them in patch release.

I think right now this bug does not occur in practice because the first check (!util.isCompilableExtension) returns false, so the if statement never triggers.

Correct, if a folder is specified in filenames, users should always specified --extensions otherwise babel doesn't know it should compile foo/*.ts. The filenames.includes is working only for babel foo.ts cases, since --extensions ts is implied from the arguments.

there doesn't seem to be any enforcement of ordering

It could happen, though we have used awaitFinish in chokidar so that every compilation should has a 50ms time window during which the file can be considered unchanged. But we can't ensure a compilation task can be finished in 50ms. I suggest we can explore them in a separate PR beside the changes we mentioned above.

vedantroy · 2020-06-29T03:39:05Z

@JLHwung -- the PR is ready for review now!

~~As a sidenote, this PR has a video doc attached to it. You can view it here.~~

~~You can also download the client here. Using the client, you can do vdoc blame <file name> to get a line-by-line view of a file and its corresponding video docs.~~

Typing vdoc blame on any file modified in this PR will show the video doc linked above as well as the lines of code it corresponds to. I hope this will make it easier for future contributors to modify this part of the codebase.

vedantroy · 2020-08-16T00:39:30Z

@JLHwung any updates on this? I do think this would be helpful for plugin/macro authors. At the same time, I understand that the Babel core team is busy, so if right now's not a good time, I can check back later 😃

goatandsheep · 2020-11-03T16:37:29Z

@sosukesuzuki anywhere we can improve on this to get this out?

vedantroy · 2020-11-03T21:01:16Z

@goatandsheep I'm not a maintainer, just the PR author, but it seems good to add some tests to verify this new feature works. I would do it myself if I had the time.

MauriceArikoglu · 2021-11-16T21:22:11Z

Any news when this will hit prod? @JLHwung @vedantroy @goatandsheep @nicolo-ribaudo

JLHwung · 2021-11-17T14:37:59Z

@vedantroy @MauriceArikoglu Sorry I completely forgot this PR. I will rebase on the latest main and it should be ready for review.

It's not sufficient to know the file paths of all external dependencies. Babel needs to know which source files correspond to which external dependencies and vice-versa.

Video-Doc-Id[b0]: babel/babel/1

MauriceArikoglu · 2021-11-22T11:41:03Z

@JLHwung thank you! Can I be of any help in the review process?

JLHwung · 2021-11-22T21:10:49Z

packages/babel-cli/src/babel/util.ts

+        return await callback(null);
+      } else {
+        for (const dependent of externalFileDeps.get(filePath)) {
+          await callback(dependent);


This can be improved by Promise.all: the external deps can be compiled in parallel.

@JLHwung can it actually? what if a dependency depends on another dependency?

If thats not possible, you are right. It should be changed to Promise.all @vedantroy

I think this should be changed to Promise.all (the API doesn't support a chain of deps), alas I don't have tons of bandwidth to do this right now so if @JLHwung wants to change it, that'd be great.

MauriceArikoglu · 2021-11-25T21:17:26Z

🚀 🚀 🚀

Thanks to @JLHwung and @vedantroy!

nicolo-ribaudo

I downloaded this locally to play a bit with it, and there are a few changes that we need to do:

api.addExternalDependency should only take a single parameter: the dependency path. It should already know the path of the current file.
the dependencies and externalDependencies set contain the same data, they are just inverted; I don't understand why we need both of them.

What should happen when the dependencies of a file change? Assume we have this plugin, which replaces the INLINE_ME variable with the file contents.

function inlineFilePlugin(api, { inlineFile }) {
  const { types: t } = api;

  api.addExternalDependency(inlineFile, "./demo.js");

  return {
    visitor: {
      Identifier(path) {
        if (path.node.name === "INLINE_ME") {
          path.replaceWith(t.stringLiteral(readFileSync(inlineFile, "utf8")));
        }
      },
    },
  };
}

the user might be running it like this:

babel.transformFileSync("./demo.js", {
  plugins: [[inlineFilePlugin, { inlineFile: "./file1.txt" }]],
});

The user has a watcher that uses babel.getExternalDependencies() to re-trigger their compiler step. Whenever file1.txt changes, it re-compiles demo.js.

At some point the config changes to { inlineFile: "./file2.txt" }: when file1.txt changes, it should not re-compile demo.js anymore because it's not a dependency anymore.

I was going to propose fixing this by re-setting demo.js's dependencies whenever it's compiled, but then I realized that the user might want to compile both with { inlineFile: "./file1.txt" } and { inlineFile: "./file2.txt" } for different build targets that run in "parallel".

The solution to all of this is to not store dependencies in a shared global, but to return them in a Set in the result of babel.transform*: Babel integrations are then free that set however they want, avoiding:

global state that leaks across unrelated build pipelines
"phantom" dependencies that should not be marked as dependencies anymore.

I can do these changes if you want; I also want to add tests.

nicolo-ribaudo · 2021-12-20T00:34:38Z

I opened an alternative PR: #14065

JLHwung reviewed Jun 23, 2020

View reviewed changes

packages/babel-core/src/config/full.js Outdated Show resolved Hide resolved

JLHwung reviewed Jun 23, 2020

View reviewed changes

packages/babel-core/src/index.js Outdated Show resolved Hide resolved

JLHwung added pkg: core PR: New Feature 🚀 A type of pull request used for our changelog categories labels Jun 23, 2020

vedantroy force-pushed the external-deps branch from cc27b60 to 22f79e4 Compare June 26, 2020 07:18

vedantroy changed the title ~~[Draft] Allow plugins to indicate dependencies on random files~~ Allow plugins to indicate dependencies on random files Jun 26, 2020

vedantroy changed the title ~~Allow plugins to indicate dependencies on random files~~ [Draft] Allow plugins to indicate dependencies on random files Jun 26, 2020

vedantroy force-pushed the external-deps branch from f6a6aaa to 34971ac Compare June 27, 2020 01:28

vedantroy changed the title ~~[Draft] Allow plugins to indicate dependencies on random files~~ Allow plugins to indicate dependencies on random files Jun 27, 2020

vedantroy force-pushed the external-deps branch from 2c2975c to 8dfab48 Compare June 29, 2020 03:39

JLHwung self-requested a review August 16, 2020 01:02

unlight mentioned this pull request Sep 11, 2020

Unable to watch the changes on the source CSS file once imported unlight/babel-plugin-postcss#1

Open

TJTorola mentioned this pull request Oct 6, 2020

.env file variables are cached goatandsheep/react-native-dotenv#75

Closed

goatandsheep approved these changes Oct 7, 2020

View reviewed changes

nicolo-ribaudo self-requested a review October 14, 2020 22:00

Arthie mentioned this pull request Nov 26, 2020

Font weight not generated with removeFallbacks plugin Arthie/xwind#18

Closed

Arthie mentioned this pull request Dec 19, 2020

Changes to the tailwind.config.js not taking effect in the xw types Arthie/xwind#24

Closed

mosesoak mentioned this pull request Jun 15, 2021

Gatsby caching issue ben-rogerson/twin.macro#11

Closed

MauriceArikoglu mentioned this pull request Nov 16, 2021

How to force recompile? kentcdodds/babel-plugin-preval#19

Open

MauriceArikoglu mentioned this pull request Nov 16, 2021

[loader] Recompile problem (babel cache) evenchange4/graphql.macro#6

Open

vedantroy added 11 commits November 17, 2021 10:17

Enable plugins to mark external dependencies

266d4a1

Add missing export

3b04c0f

Detailed external dependency tracking

f048795

It's not sufficient to know the file paths of all external dependencies. Babel needs to know which source files correspond to which external dependencies and vice-versa.

Integrate external dependency tracking with watch mode

b9c073d

Formatting/naming

3bb337c

Only watch files when watch flag is set

d500b8c

Remove outdated addExternalDependency

8969803

Properly register external dependencies

ceb8667

Only watch external deps if watch cli option is present

dcbb5ae

More comments

344860b

remove unnecessary else

32cf26a

Video-Doc-Id[b0]: babel/babel/1

JLHwung force-pushed the external-deps branch from 8dfab48 to 32cf26a Compare November 17, 2021 15:30

JLHwung reviewed Nov 22, 2021

View reviewed changes

meduzen-immoweb mentioned this pull request Nov 25, 2021

Changes to .browserslistrc does not invalidate cache babel/babel-loader#690

Open

JLHwung approved these changes Nov 25, 2021

View reviewed changes

nicolo-ribaudo added this to the v7.17.0 milestone Nov 25, 2021

nicolo-ribaudo requested changes Dec 18, 2021

View reviewed changes

nicolo-ribaudo mentioned this pull request Dec 20, 2021

Allow plugins/presets to indicate external dependencies #14065

Merged

raqso approved these changes Jan 21, 2022

View reviewed changes

nicolo-ribaudo closed this in #14065 Feb 2, 2022

github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label May 5, 2022

github-actions bot locked as resolved and limited conversation to collaborators May 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow plugins to indicate dependencies on random files #11741

Allow plugins to indicate dependencies on random files #11741

vedantroy commented Jun 23, 2020 •

edited by gitpod-io bot

codesandbox-ci bot commented Jun 23, 2020 •

edited

babel-bot commented Jun 23, 2020 •

edited

vedantroy commented Jun 23, 2020 •

edited

JLHwung commented Jun 23, 2020

vedantroy commented Jun 24, 2020 •

edited

JLHwung commented Jun 24, 2020

vedantroy commented Jun 29, 2020 •

edited

vedantroy commented Aug 16, 2020 •

edited

goatandsheep commented Nov 3, 2020

vedantroy commented Nov 3, 2020 •

edited

MauriceArikoglu commented Nov 16, 2021

JLHwung commented Nov 17, 2021

MauriceArikoglu commented Nov 22, 2021

JLHwung Nov 22, 2021

MauriceArikoglu Nov 23, 2021 •

edited

vedantroy Nov 26, 2021 •

edited

MauriceArikoglu commented Nov 25, 2021

nicolo-ribaudo left a comment

nicolo-ribaudo commented Dec 20, 2021

Allow plugins to indicate dependencies on random files #11741

Allow plugins to indicate dependencies on random files #11741

Conversation

vedantroy commented Jun 23, 2020 • edited by gitpod-io bot

codesandbox-ci bot commented Jun 23, 2020 • edited

babel-bot commented Jun 23, 2020 • edited

vedantroy commented Jun 23, 2020 • edited

JLHwung commented Jun 23, 2020

vedantroy commented Jun 24, 2020 • edited

JLHwung commented Jun 24, 2020

vedantroy commented Jun 29, 2020 • edited

vedantroy commented Aug 16, 2020 • edited

goatandsheep commented Nov 3, 2020

vedantroy commented Nov 3, 2020 • edited

MauriceArikoglu commented Nov 16, 2021

JLHwung commented Nov 17, 2021

MauriceArikoglu commented Nov 22, 2021

JLHwung Nov 22, 2021

Choose a reason for hiding this comment

MauriceArikoglu Nov 23, 2021 • edited

Choose a reason for hiding this comment

vedantroy Nov 26, 2021 • edited

Choose a reason for hiding this comment

MauriceArikoglu commented Nov 25, 2021

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

nicolo-ribaudo commented Dec 20, 2021

vedantroy commented Jun 23, 2020 •

edited by gitpod-io bot

codesandbox-ci bot commented Jun 23, 2020 •

edited

babel-bot commented Jun 23, 2020 •

edited

vedantroy commented Jun 23, 2020 •

edited

vedantroy commented Jun 24, 2020 •

edited

vedantroy commented Jun 29, 2020 •

edited

vedantroy commented Aug 16, 2020 •

edited

vedantroy commented Nov 3, 2020 •

edited

MauriceArikoglu Nov 23, 2021 •

edited

vedantroy Nov 26, 2021 •

edited