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 file names and formats #28

Merged
merged 1 commit into from
Jul 20, 2023
Merged

Add decision record about JavaScript file names and formats #28

merged 1 commit into from
Jul 20, 2023

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Jul 4, 2023 •
edited
Loading

Closes: #26

@colinrotherham colinrotherham linked an issue Jul 4, 2023 that may be closed by this pull request
Write up decisions on package structure and JavaScript formats in v5 #26
Closed
2 tasks
36degrees
36degrees reviewed Jul 4, 2023
View reviewed changes
decision-records/007-javascript-file-formats.md Outdated

We will use ECMAScript (ES) modules by default and only bundle JavaScript files that are considered "entry points" into GOV.UK Frontend, such as `all.mjs` and exported components (for example `accordion.mjs`).

We will deprecate the Universal Module Definition (UMD) format in a future release.
Copy link
Contributor

Choose a reason for hiding this comment

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

We talked about this, but I think we should focus decision records on things we've decided to do now, and avoid making future-looking statements.

(I'd be happy to mention it as a future possibility elsewhere in the doc, just not including it in the decision)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah this was one of @romaricpascal's decisions that we'd deprecate UMD + window globals in future

Will remove

decision-records/007-javascript-file-formats.md Outdated
1. Add prefix `.bundle` to bundled JavaScript file extensions
2. Add prefix `.min` to minified JavaScript file extensions

We will add our [GitHub release](https://github.com/alphagov/govuk-frontend/releases) JavaScript to the [`govuk-frontend` npm package](https://www.npmjs.com/package/govuk-frontend).
Copy link
Contributor

Choose a reason for hiding this comment

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

Think it's worth being clearer about what's being included from the package – as far as I recall it's just the minified JS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, our paths crossed and it was a post-sports day change to add the directory listing example 😊

Pushed up a bit more now

decision-records/007-javascript-file-formats.md Outdated
Comment on lines 24 to 26
**Date:** 2023-06-27

**Status:** accepted
Copy link
Contributor

Choose a reason for hiding this comment

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

I've raised #29 to fix this in the template, but can be bump the date and status to the top of the doc, and sentence case the status?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Spotted it, pushed 👍

decision-records/007-javascript-file-formats.md Show resolved Hide resolved
@colinrotherham colinrotherham marked this pull request as draft July 4, 2023 15:53
@colinrotherham colinrotherham marked this pull request as ready for review July 4, 2023 16:40
@colinrotherham
Copy link
Contributor Author

Bringing this back out of draft now @36degrees @claireashworth

@36degrees 36degrees mentioned this pull request Jul 5, 2023
Merged
@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 6, 2023
Closed
@colinrotherham
Copy link
Contributor Author

Thanks @36degrees

Do we need to mention that the bundled code is still full of (quite large) comment blocks?

@36degrees
Copy link
Contributor

I don't think so, unless you think it's relevant for the overall decision (i.e. if it was somehow a factor in what we decided, or is tied in the implications)?

@colinrotherham
Copy link
Contributor Author

It was flagged during, but not really related to this decision

@romaricpascal romaricpascal force-pushed the javascript-browser-compatibility branch 11 times, most recently from 2698730 to c016c51 Compare July 20, 2023 13:15
Base automatically changed from javascript-browser-compatibility to main July 20, 2023 13:16
@colinrotherham colinrotherham merged commit 7bb3071 into main Jul 20, 2023
@colinrotherham colinrotherham deleted the javascript-file-names-formats branch July 20, 2023 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@claireashworth claireashworth claireashworth approved these changes

@36degrees 36degrees 36degrees approved these changes

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 package structure and JavaScript formats in v5
3 participants
@colinrotherham @36degrees @claireashworth