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.

Sign up for GitHub

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

Jump to bottom

Docs: conflict in next/script's onLoad documentation #33402

Closed
akselinurmio opened this issue Jan 17, 2022 · 1 comment · Fixed by #36453
Closed

Docs: conflict in next/script's onLoad documentation #33402

akselinurmio opened this issue Jan 17, 2022 · 1 comment · Fixed by #36453
Assignees
@housseindjirdeh
Labels
good first issue Easy to fix issues, good for newcomers

Comments

@akselinurmio
Copy link

akselinurmio commented Jan 17, 2022 •
edited

What is the improvement or update you wish to see?

I'm afraid there's currently a conflict between the two documentation pages on next/script. The Script Component page says following:

If you are loading a script with either beforeInteractive or afterInteractive as a loading strategy, you can execute code after it has loaded using the onLoad property

However, in the API reference page on next/script, under onLoad, it says:

Note: onLoad can't be used with the beforeInteractive loading strategy.

As far as I know, the onLoad prop for beforeInteractive is not currently working and there's an open issue on that. Either way, the documentation should be in line with the current state of affairs.

Is there any context that might help us understand?

I can see the reason why a separate API documentation page for next/script exists in addition to the guide page on script optimization, but I would hope that any time either of those two are updated, the other would be checked for inconsistencies.

The inconsistency was created in #33097.

Does the docs page already exist? Please link to it.

No response
@akselinurmio akselinurmio added the Documentation Related to Next.js' official documentation. label Jan 17, 2022
@akselinurmio akselinurmio mentioned this issue Jan 17, 2022
Closed
@molebox molebox added good first issue Easy to fix issues, good for newcomers area: documentation and removed Documentation Related to Next.js' official documentation. labels Mar 7, 2022
@lfades lfades mentioned this issue Apr 25, 2022
Merged
7 tasks
@kodiakhq kodiakhq bot closed this as completed in #36453 Apr 25, 2022
kodiakhq bot pushed a commit that referenced this issue Apr 25, 2022
@lfades
Update usage paragraph of next/script onLoad (#36453)
ff140a7 
Changing the paragraph to not include `beforeInteractive` as one of the possible use cases of `onLoad`.

*Update:* Added docs for `onError` in the API reference of `next/script`.

@housseindjirdeh does `onError` also has the same limitation or is this only for `onLoad`?

Closes #33402

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [x] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [x] Make sure the linting passes by running `yarn lint`
@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@housseindjirdeh housseindjirdeh

Labels
good first issue Easy to fix issues, good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update usage paragraph of next/script onLoad lfades/next.js
3 participants
@akselinurmio @housseindjirdeh @molebox