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

Remove src to lib rewrites on package.json files #9612

Merged
merged 12 commits into from Apr 30, 2024

Conversation

yamadapc
Copy link
Contributor

@yamadapc yamadapc commented Apr 3, 2024

This commit modifies the strategy to local src/lib rewrites, so that the package.json files are never modified.

Intead, the package.json files always point at bin/...; which in turn requires lib/bin.js.

During development, lib/bin.js can just require src/bin.js and rely on require.extensions register. But on distribution lib/** can be pre-built.

By maintaining a stable bin/... entry-point, the following benefits are achieved:

  • The git workspace does not become dirty after a build
  • It's not possible to get into a state where node_modules/.bin/parcel points at a non existant lib/ file, which can only be fixed by removing node_modules and yarn installing again

✔️ PR Todo

  • Verify distributed assets from CI

@yamadapc yamadapc marked this pull request as ready for review April 3, 2024 00:42
pyamada-atlassian and others added 5 commits April 4, 2024 18:58
This commit modifies the strategy to local src/lib rewrites, so that the
package.json files are never modified.

Intead, the package.json files always point at `bin/...`; which in turn
requires `lib/bin.js`.

During development, `lib/bin.js` can just require `src/bin.js` and rely on
require.extensions register. But on distribution `lib/**` can be pre-built.

By maintaining a stable `bin/...` entry-point, the following benefits are
achieved:

* The git workspace does not become dirty after a build
* It's not possible to get into a state where `node_modules/.bin/parcel` points
  at a non existant `lib/` file, which can only be fixed by removing
  node_modules and yarn installing again
@mattcompiles
Copy link
Contributor

@yamadapc Is my understanding correct that to use Parcel via parcel link you'll now need to run yarn lerna run dev:prepare beforehand? Otherwise there won't be anything present in lib/bin.js?

@yamadapc
Copy link
Contributor Author

yamadapc commented Apr 9, 2024

@yamadapc Is my understanding correct that to use Parcel via parcel link you'll now need to run yarn lerna run dev:prepare beforehand? Otherwise there won't be anything present in lib/bin.js?

That's right. We could add a postinstall hook or such that runs dev:prepare on local.

But IMO this is better because before you also would get corrupted workspaces on builds linked or otherwise.

I have a feeling at the moment that you can run yarn build once and after that your workspace is unable to build itself again. Also there are those dirty files & yarn over a dirty workspace will break node_modules links forever so then you need to clean-up.

I'm running benchmarks in https://github.com/parcel-bundler/parcel-benchmark-action with prod. builds and in the current setup the workspace gets corrupted every time I build.

So this change would help; there also is no way to commit the dirty files.

The specific workflow for parcel-link might change very slightly; but it should be alright.

@yamadapc yamadapc enabled auto-merge (squash) April 30, 2024 02:49
@yamadapc yamadapc disabled auto-merge April 30, 2024 03:30
@yamadapc yamadapc enabled auto-merge (squash) April 30, 2024 03:30
@yamadapc yamadapc merged commit 0eaf926 into parcel-bundler:v2 Apr 30, 2024
16 of 17 checks passed
@yamadapc yamadapc deleted the issue/rewrite-lib-fixes branch April 30, 2024 04:00
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.

None yet

4 participants