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

Add decision record about JavaScript browser compatibility #25

Merged
merged 1 commit into from
Jul 20, 2023
Merged

Add decision record about JavaScript browser compatibility #25

merged 1 commit into from
Jul 20, 2023

Conversation

romaricpascal
Copy link
Member

@romaricpascal romaricpascal commented Jun 30, 2023 •
edited
Loading

This records the decision following this week's discussion where we settled on what our approach would be to ensure how we'll make our JavaScript compatible with the browsers we set out to support

@romaricpascal romaricpascal force-pushed the javascript-browser-compatibility branch from 0df8c3a to b5a86f8 Compare June 30, 2023 10:00
@romaricpascal romaricpascal marked this pull request as ready for review June 30, 2023 11:11
@36degrees 36degrees mentioned this pull request Jun 30, 2023
Merged
@36degrees 36degrees linked an issue Jul 4, 2023 that may be closed by this pull request
Write up decisions on JavaScript browser compatibility in v5 #27
Closed
2 tasks
@36degrees 36degrees force-pushed the javascript-browser-compatibility branch from b5a86f8 to 0414f02 Compare July 6, 2023 08:20
@colinrotherham colinrotherham mentioned this pull request Jul 11, 2023
Merged
@romaricpascal romaricpascal force-pushed the javascript-browser-compatibility branch 3 times, most recently from 7a451f4 to 1014c34 Compare July 14, 2023 12:58
colinrotherham
colinrotherham reviewed Jul 14, 2023
View reviewed changes
decision-records/006-javascript-compatibility.md Outdated Show resolved Hide resolved
decision-records/006-javascript-compatibility.md Outdated Show resolved Hide resolved
decision-records/006-javascript-compatibility.md Show resolved Hide resolved
decision-records/006-javascript-compatibility.md Outdated Show resolved Hide resolved
decision-records/006-javascript-compatibility.md Outdated Show resolved Hide resolved
decision-records/006-javascript-compatibility.md Outdated Show resolved Hide resolved
decision-records/006-javascript-compatibility.md Outdated Show resolved Hide resolved
decision-records/006-javascript-compatibility.md Outdated Show resolved Hide resolved
decision-records/006-javascript-compatibility.md Outdated Show resolved Hide resolved
@romaricpascal romaricpascal force-pushed the javascript-browser-compatibility branch 2 times, most recently from 2ec3d17 to 5a296f8 Compare July 17, 2023 10:20
@romaricpascal
Copy link
Member Author

@colinrotherham I've updated the record based on your feedback. @claireashworth please let me know if there's any content updates to make before we can merge it 😊

colinrotherham
colinrotherham reviewed Jul 17, 2023
View reviewed changes
decision-records/006-javascript-compatibility.md Outdated Show resolved Hide resolved
decision-records/006-javascript-compatibility.md Outdated Show resolved Hide resolved
decision-records/006-javascript-compatibility.md Outdated Show resolved Hide resolved
decision-records/006-javascript-compatibility.md Outdated Show resolved Hide resolved
decision-records/006-javascript-compatibility.md Outdated Show resolved Hide resolved
decision-records/006-javascript-compatibility.md Outdated Show resolved Hide resolved
decision-records/006-javascript-compatibility.md Outdated Show resolved Hide resolved
decision-records/006-javascript-compatibility.md Outdated Show resolved Hide resolved
decision-records/006-javascript-compatibility.md

Using a dynamic `import()` call would allow to handle syntax errors when parsing GOV.UK Frontend (via a `.catch` on the promise, or a `try/catch` block, depending on the browsers you'd want to support).

This complicates the integration for services as it requires more JavaScript knowledge to understand which piece of code will run or not if GOV.UK Frontend fails to parse. It also complicates the import of multiple individual components, requiring to make sure the `import()` calls run in parallel to not hinder performance (on top of the individual components not being discoverable by [the browsers' preload-scanner](https://web.dev/preload-scanner/)), but handle the potential failure of these calls appropriately.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to mention the opposite?

That as our JavaScript takes longer and longer to run, we'll delay the initialisation of components that run last (as they get queried, looped and have init() run)

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you mean by "the opposite"? I'm sure I understand what leads to "our JavaScript takes longer and longer to run" (only thing I come to mind is the growing number of transformations that will make our file size grow).

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I was commenting on "import() calls run in parallel"

As in, do we need to comment on "the opposite" benefits of Promises?

  1. By supporting sequential .init(), we can simplify integration by avoiding Promises
  2. By supporting concurrent .init(), we can avoid render blocking from slow forEach()

Or shall we remove anything that can be moved to an API decision record?

decision-records/006-javascript-compatibility.md Outdated Show resolved Hide resolved
@romaricpascal romaricpascal force-pushed the javascript-browser-compatibility branch 3 times, most recently from 5a5aa59 to 84e0f3d Compare July 17, 2023 15:22
colinrotherham
colinrotherham approved these changes Jul 17, 2023
View reviewed changes
Copy link
Contributor

@colinrotherham colinrotherham left a comment

Choose a reason for hiding this comment

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

Based on how light our previous decisions were, I'm sure a few sections could be reduced to fewer words. But otherwise, looks good to me

@romaricpascal romaricpascal force-pushed the javascript-browser-compatibility branch from fc42b93 to 5122ce9 Compare July 20, 2023 11:12
36degrees
36degrees approved these changes Jul 20, 2023
View reviewed changes
Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

Looks ace, thanks for making those last few changes ❤️

@romaricpascal romaricpascal force-pushed the javascript-browser-compatibility branch from 146d21b to 2698730 Compare July 20, 2023 13:13
@romaricpascal @36degrees @colinrotherham
Add decision record about JavaScript browser compatibility
c016c51 
Co-authored-by: Oliver Byford <oliver.byford@digital.cabinet-office.gov.uk>
Co-authored-by: Colin Rotherham <colin.rotherham@digital.cabinet-office.gov.uk>
@romaricpascal romaricpascal force-pushed the javascript-browser-compatibility branch from 2698730 to c016c51 Compare July 20, 2023 13:15
@romaricpascal romaricpascal merged commit 18e1a1d into main Jul 20, 2023
@romaricpascal romaricpascal deleted the javascript-browser-compatibility branch July 20, 2023 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@36degrees 36degrees 36degrees approved these changes

@colinrotherham colinrotherham colinrotherham approved these changes

@claireashworth claireashworth Awaiting requested review from claireashworth

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Write up decisions on JavaScript browser compatibility in v5
3 participants
@romaricpascal @36degrees @colinrotherham