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

Chore: Backport recent refactorings #1648

Merged
merged 7 commits into from Feb 2, 2022
Merged

Chore: Backport recent refactorings #1648

merged 7 commits into from Feb 2, 2022

Conversation

rschristian
Copy link
Member

@rschristian rschristian commented Feb 1, 2022

What kind of change does this PR introduce?

Minor refactors

Did you add tests for your changes?

Existing cover

Summary

Backport of misc changes made in #1466/#1608 and #1645. I don't want those PRs to get completely out-of-hand and impossible to review, so I'm extracting out some of the more less-focused changes.

  • Added a jsconfig.json for better intellisense. Seems like a pretty handy addition, caught a bunch of minor issues already.
  • Re-enables the build output checking from chore: update html-webpack-plugin #1466
  • Removes non-functional missing node modules check (read below)
  • Public path is ensured to be normalized for service workers
    • publicPath = http://some-url.com/foo -> http://some-url.com/foosw.js without this, which is no good.

Does this PR introduce a breaking change?

No

@changeset-bot
Copy link

changeset-bot bot commented Feb 1, 2022

🦋 Changeset detected

Latest commit: ab0a238

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
preact-cli Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Comment on lines -20 to -35
compiler.hooks.emit.tapAsync('CliDevPlugin', (compilation, callback) => {
let missingDeps = compilation.missingDependencies;
let nodeModulesPath = resolve(__dirname, '../../../node_modules');

// ...tell webpack to watch node_modules recursively until they appear.
if (
Array.from(missingDeps).some(
file => file.indexOf(nodeModulesPath) !== -1
)
) {
compilation.contextDependencies.push(nodeModulesPath);
}

callback();
});

Copy link
Member Author

Choose a reason for hiding this comment

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

This was adopted from CRA's WatchMissingNodeModulesPlugin, however, it seems to have been broken for a few years. This path that it watches resolves to <project-name>/node_modules/preact-cli/node_modules which is pretty pointless, as nothing will ever exist/be added to that.

Rather than correct it, I've elected to remove it after seeing it was recently cut from CRA in v5. It apparently was having a rather large performance impact which they either couldn't solve or decided it wasn't worth solving. Not entirely sure which.

I figure there's probably no reason to doubt the performance issues claims, so I cut it.

@rschristian rschristian marked this pull request as ready for review February 2, 2022 05:18
@rschristian rschristian requested a review from a team as a code owner February 2, 2022 05:18
@rschristian rschristian changed the title Chore/backport Chore: Backport recent refactorings Feb 2, 2022
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

2 participants