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

refactor: Remove --preload flag #1737

Merged
merged 3 commits into from
Aug 18, 2022
Merged

refactor: Remove --preload flag #1737

merged 3 commits into from
Aug 18, 2022

Conversation

rschristian
Copy link
Member

What kind of change does this PR introduce?

Removal of a feature

Did you add tests for your changes?

Existing cover

Summary

Removes --preload flag from peact build.

Upon writing #1731 I came to notice that our system for creating preload tags was quite unsound. We rely entirely upon the assumption that the user's directory structure matches their router perfectly. If it doesn't, preload tags will not be generated correctly.

For example, assume the following directory structure:

src
--routes
----foo.js

This creates a chunk route-foo and generates a load manifest looking something like the following:

{
    "/foo": {
        "bundle.abcde.css": {
		"type": "style",
		"weight": 1
	},
	"bundle.abcde.js": {
		"type": "script",
		"weight": 1
	},
	"route-foo.chunk.abcde.js": {
		"type": "script",
		"weight": 0.9
	},
	"route-foo.chunk.abcde": {
		"type": "style",
		"weight": 0.9
	}
}

The problem here is that we've just equated routes/foo.js with /foo, without actually acknowledging what the router says of the matter.

import Foo from 'routes/foo.js';

<Router>
    <Foo path="/bar" />
</Router>

In this situation, preload tags will never be created as the chunks are attached to /foo which is never prerendered.

In a more realistic scenario, imagine we have routes/about and /about-us. Preload won't work as the route doesn't match the directory structure perfectly.

Fundamentally this feature is majorly flawed, and as such I'm removing it. I'll try to reimplement it if possible later, but I really don't think it can stay in this state. Even if a user knows exactly how this works (and that's asking a lot), it's way too easy to accidentally break.

Does this PR introduce a breaking change?

Yes.

The --preload flag is removed. There's no migration path at the time of writing, though hopefully that will change in the future.

@rschristian rschristian requested a review from a team as a code owner August 18, 2022 03:10
@changeset-bot
Copy link

changeset-bot bot commented Aug 18, 2022

🦋 Changeset detected

Latest commit: 5e79f26

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 Major

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

@rschristian rschristian force-pushed the refactor/remove-preload branch from a5fb166 to 5e79f26 Compare August 18, 2022 03:13
@rschristian rschristian merged commit 4953687 into next Aug 18, 2022
@rschristian rschristian deleted the refactor/remove-preload branch August 18, 2022 03:42
rschristian added a commit that referenced this pull request Nov 18, 2022
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Dec 13, 2022
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Dec 13, 2022
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Dec 13, 2022
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Dec 13, 2022
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Dec 13, 2022
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Dec 13, 2022
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Dec 13, 2022
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Dec 20, 2022
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Dec 20, 2022
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Dec 20, 2022
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Dec 20, 2022
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Dec 22, 2022
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Dec 24, 2022
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Dec 24, 2022
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Dec 24, 2022
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Jan 6, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Jan 6, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Jan 6, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Jan 6, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Jan 6, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Jan 7, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Jan 7, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Jan 8, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Jan 9, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Jan 9, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Jan 9, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Jan 9, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Jan 16, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Jan 31, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Jan 31, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Jan 31, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Jan 31, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Feb 2, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Oct 10, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Oct 10, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Oct 10, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Oct 10, 2023
* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload
rschristian added a commit that referenced this pull request Oct 10, 2023
* Chore: Updating html-webpack-plugin to v4 (#1608)

* chore: Correcting changeset

* feat: webpack v5 (#1645)

* feat: webpack-v5

* test: Update tests

* docs: Updating docs to reflect changes

* chore: Removing optimize-plugin local patch

* refactor: Determine CSS module by filename, not directory (#1714)

* refactor: Determine CSS module by filename, not directory

* docs: Adding changeset

* refactor: Project creation pull from templates 'main' branch

* test: Updating tests

* revert: Accidentally removed log message

* refactor: Extracting project creation out to own package (#1708)

* refactor: Extracting out project creation to own package

* docs: Adding changeset

* refactor: Misc edits to common deps & workspace helper scripts

* test: Fixing 'create' test suite

* ci: Increase minimum Node version

* chore: Fix typos

* docs: Update ReadMe instructions

* chore: Better (and untaken) package name

* docs: Update changeset

* test: Update 'create' test suite & build output hashes

* docs: Expounding upon changeset

* refactor: Drop Preact 8 support (#1716)

* refactor: Drops support for Preact v8

* docs: Adding changeset

* refactor: Switch to automatic runtime JSX transform (#1725)

* refactor: Switch over to automatic runtime transform for JSX

* test: Removing `h` imports from test suite

* docs: Adding changeset

* docs: Adding readme for create-cli (#1728)

* docs: Adding readme for create-cli

* feat: Initial publish

* fix: Correcting broken readme links

* refactor: Disable hashing ssr-build's css (#1733)

* refactor: Disable hashing ssr-build's css

* docs: Adding changeset

* test: Fix unrelated change from altered template

* refactor: Remove `--preload` flag (#1737)

* refactor: Removes `--preload` flag & functionality

* docs: Adding changeset

* docs: Updating readme for removal of preload

* refactor: Separate internal concepts of config and env (#1734)

* refactor: Separate internal concepts of config and env

* docs: Adding changeset

* refactor: prune flags (#1752)

* refactor: Revise inconsistent, unused, and deprecated flags

* docs: Adding changeset

* feat: Improved errors and warnings (#1758)

* refactor: Better root error handling

* refactor: Switch prerender plugin to be async for error gen

* refactor: Simplify & improve compilation messages

* refactor: Bump Node version, remove `src` arg in build & watch, and misc housekeeping (#1753)

* refactor: Drop rimraf for built-in fs.rm

* refactor: Drop src argument in build & watch cmds

* refactor: Extract info cmd from CLI entrypoint

* refactor: Remove update-notifier

* refactor: Switch from fs.promises to fs/promises

* docs: Adding changeset

* refactor: Move EJS into user templates (`template.html`) (#1768)

* refactor: Merging EJS templates and switcing preferred template extension to .ejs

* test: Fixing test suite to reflect latest changes

* chore: Cleaning

* docs: Updating ReadMe

* docs: Adding changeset

* refactor: Add error message for <% preact.(head|body)End %>

* chore: Publishing cli & create-cli

* Refactor: Enable Prefresh by Default (#1772)

* refactor: Enables HMR via Prefresh by default

* docs: Adding changeset

* test: Updating tests to reflect changes to templates

* refactor: Remove Critters (perhaps temporarily) (#1778)

* refactor: Remove Critters (perhaps temporarily)

* docs: Adding changeset

* chore: Publish v4.0.0-next.2

* refactor: Vendor Critters so that 7c811ac can be reverted (#1780)

* docs: Remove CLI beta note from readme

* docs: Bump minimum Node version listed in readme
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.

1 participant