add invalidateOnEnvChange to resolver

[tonyhallett]

↪️ Pull Request

Fixes #8094

Added unit tests in PathRequest.test.js
To be able to test from the PathRequest I have pre-populated the build cache with a ParcelConfig that will be returned later. This allowed providing resolvers in the test.

Hopefully this is sufficient and that it is not necessary to create an integration test in https://github.com/parcel-bundler/parcel/blob/v2/packages/core/integration-tests/test/plugin.js

✔️ 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

[tonyhallett]

an alternative is to change PathRequest.run to

async function run({input, api, options}: RunOpts) {
  let configResult = nullthrows(
    await api.runRequest<null, ConfigAndCachePath>(createParcelConfigRequest()),
  );
  let config = getCachedParcelConfig(configResult, options);
  

// move the logic into here
return new PathRequestRunner(config,api, options,input.dependency);

[tonyhallett]

Will check out why the test is failing

[tonyhallett]

Must be platform based. Passes on windows. Will update with an alternative.

[mischnic]

We don't really use unit tests for most things. For example like this one:

parcel/packages/core/integration-tests/test/plugin.js

Line 90 in 8981c88

it('invalidate the cache based on loadConfig in a packager', async function () {

So there'd be a dummy resolver that uses invalidateOnEnvChange and then the test would change the variable and rebuild

[tonyhallett]

Yes I had seen these tests. The approach I was using made it simpler to cover the paths taken, although it did require hacking the cache.

From what I saw there is currently not a single intergration test for resolver plugins peforming any kind of invalidation.

I am happy to do a single integration test for when there is a single resolver plugin that does resolve and invalidates some environment variables. This does not cover all paths.

[mischnic]

From what I saw there is currently not a single intergration test for resolver plugins peforming any kind of invalidation.

There are a number of those:

parcel/packages/core/integration-tests/test/cache.js

Line 2716 in 8981c88

describe('resolver', function () {

[tonyhallett]

I realised I had discounted the default resolver plugin, in particular the NodeResolver, late last night. That I knew invalidated files from deriving from it for my alias plugin. So apologies for that.

Which leads to the question. Shall incorporate my idea into NodeResolver ? An alias-file-path environment variable / alias key values ?

[mischnic]

Shall incorporate my idea into NodeResolver ? An alias-file-path environment variable / alias key values ?

I'm not aware of some standardized env-based alias in package.json. So no, there's no need to find an "artificial" usecase for this in the NodeResolver.

[tonyhallett]

Perhaps then you could tell me if Parcel supports my use case.

I have a react app. I require 2:production builds, one with react-dom aliased for profiling. There are two different index.html sharing the same code. I can script the package.json alias but then this invalidates the other build as well as the development build. With an environment variable that will not change per entry ( I have 3 different entries ) then I believe I do not get this behaviour.

Perhaps I have this wrong ?

[tonyhallett]

I should have said incorporate my idea into the default resolver plugin.

[mischnic]

Have you tried aliasing react-dom to some local file which does

if (process.env.PROFILING) {
  module.exports = require("react-dom/profiling");
} else {
  module.exports = require("react-dom");
}

[tonyhallett]

This does not work

require("react-dom/profiling");

I cannot remember the error message exactly but I think it has to do with not providing a further alias - scheduler ?

[mischnic]

In the most simple case, that will just resolve to react-dom/profiling.js which does exist: https://unpkg.com/browse/react-dom@18.1.0/profiling.js

Anyway, would be great if you could do this:

a single integration test for when there is a single resolver plugin that does resolve and invalidates some environment variable

[tonyhallett]

In the most simple case, that will just resolve to react-dom/profiling.js which does exist: https://unpkg.com/browse/react-dom@18.1.0/profiling.js

With the version of react I am tied to ^17.0.2 - requiring react-dom/profiling.js will

Uncaught` Error: Minified React error #302; visit https://reactjs.org/docs/error-decoder.html?invariant=302 for the full message or use the non-minified dev environment for full errors and additional helpful warnings.

From the link

It is not supported to run the profiling version of a renderer (for example, react-dom/profiling) without also replacing the scheduler/tracing module with scheduler/tracing-profiling. Your bundler might have a setting for aliasing both modules. Learn more at https://reactjs.org/link/profiling

So parcel does not support conditional aliases. Manually changing package.json / using a script and invalidating is the only solution.

Anyway. I'll add the integration test shortly.

[mischnic]

Thanks!

[tonyhallett]

@mischnic Please can this get merged. Thanks