fix(create-quasar): remove unused ref import in Options API template (#18227)#18228
fix(create-quasar): remove unused ref import in Options API template (#18227)#18228arosstale wants to merge 2 commits intoquasarframework:devfrom
ref import in Options API template (#18227)#18228Conversation
When scaffolding with Options API (`sfcStyle === 'options'`), the
MainLayout.vue template imports `ref` from 'vue' but never uses it —
Options API uses `data()` + `methods` instead. The import only applies
to the Composition API blocks above it.
This causes a build error immediately after scaffolding:
import { defineComponent, ref } from 'vue'
^^^ — unused
Remove the unused `ref` from the Options API block in both
js-vite-2 and js-webpack-4 templates.
Fixes quasarframework#18227
When 'noopener' is present in the window features (which is the default), `window.open()` intentionally returns `null` even though the window was successfully opened — the browser withholds the reference for security. Checking `win === null` as a failure signal is therefore incorrect and causes `rejectFn` to always fire on successful opens. Fix: only call `reject` when noopener is not in effect, so null can be treated as a genuine blocked-popup signal. Fixes quasarframework#18221
There was a problem hiding this comment.
Pull request overview
This PR aims to fix a linting/build error in scaffolded Quasar projects by removing an unused ref import from the Options API template in MainLayout.vue. When users scaffold a project with Options API, the generated code imports ref from Vue but never uses it (Options API uses data() instead of ref()), causing immediate lint errors.
However, the PR also includes unrelated and problematic changes to ui/src/utils/open-url/open-url.js that are not mentioned in the PR description. These changes introduce a bug based on an incorrect assumption about window.open() behavior with the noopener flag.
Changes:
- Removed unused
refimport from Options API template blocks in MainLayout.vue (js-vite-2 and js-webpack-4) - Added logic to conditionally call
reject()based onnoopenersetting in open-url.js (unrelated to PR purpose and contains a bug)
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| create-quasar/templates/app/quasar-v2/js-vite-2/BASE/src/layouts/MainLayout.vue | Removed unused ref import from Options API template block - correct fix |
| create-quasar/templates/app/quasar-v2/js-webpack-4/BASE/src/layouts/MainLayout.vue | Removed unused ref import from Options API template block - correct fix |
| ui/src/utils/open-url/open-url.js | Unrelated changes not mentioned in PR description; contains incorrect logic based on false assumption about noopener behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // When "noopener" is set (default), window.open() intentionally returns null | ||
| // even when the window was successfully opened. Only reject if noopener is | ||
| // not in effect, so that the null return can be treated as a blocked popup. | ||
| const cfg = Object.assign({ noopener: true }, windowFeatures) | ||
| const hasNoopener = cfg.noopener === true | ||
|
|
||
| const win = open(url, '_blank', parseFeatures(windowFeatures)) | ||
|
|
||
| if (win) { | ||
| Platform.is.desktop && win.focus() | ||
| return win | ||
| } | ||
| else { | ||
| else if (hasNoopener === false) { |
There was a problem hiding this comment.
This change to open-url.js is not mentioned in the PR description. The PR description states it only fixes unused ref imports in Options API templates, but this file contains logic changes to the window opening behavior.
If this is an intentional fix, it should be:
- Mentioned in the PR description
- Possibly split into a separate PR since it addresses a different issue
If this is an accidental inclusion from another branch, it should be removed from this PR.
| // When "noopener" is set (default), window.open() intentionally returns null | ||
| // even when the window was successfully opened. Only reject if noopener is | ||
| // not in effect, so that the null return can be treated as a blocked popup. | ||
| const cfg = Object.assign({ noopener: true }, windowFeatures) | ||
| const hasNoopener = cfg.noopener === true |
There was a problem hiding this comment.
The comment states that window.open() returns null when noopener is set, but this is incorrect. According to the HTML specification and browser behavior, window.open() returns a WindowProxy object when the window opens successfully, regardless of the noopener setting. It only returns null if the popup is blocked by the browser's popup blocker.
The noopener feature prevents the new window from accessing window.opener, but does not affect the return value of window.open(). This logic change may prevent legitimate popup blocking detection from working correctly.
|
It seems you forgot to separate a commit from #18229. So, this PR contains a commit from another PR. So, please remove it. |
What kind of change does this PR introduce?
Does this PR introduce a breaking change?
The PR fulfills these requirements:
devbranchProblem
When scaffolding a project with Options API (
sfcStyle === 'options'), the generatedMainLayout.vueimportsreffrom'vue'but never uses it. Options API components usedata()andmethods—refis only needed in Composition API.This causes a lint/build error immediately after running
npm init quasar@latest:Observed when using both
quasar-cli-viteandquasar-cli-webpackflavours.Fix
Remove
reffrom the import in theoptionstemplate block. Thecompositionandcomposition-setupblocks above it correctly keep theirrefimports.Files changed:
create-quasar/templates/app/quasar-v2/js-vite-2/BASE/src/layouts/MainLayout.vuecreate-quasar/templates/app/quasar-v2/js-webpack-4/BASE/src/layouts/MainLayout.vueOther information: Affects any user scaffolding a new Quasar project with Options API selected.