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

feat(html): support custom meta element creation #308

Merged
merged 2 commits into from
Apr 26, 2020

Conversation

mseeley
Copy link
Contributor

@mseeley mseeley commented Apr 14, 2020

This allows for convenient creation of meta elements without reimplementing all template function responsibilities.

Rollup Plugin Name: html

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.

List any relevant issue numbers: n/a

Description

The html plugin doesn't allow for injection of meta elements without re-implementing all of the template function responsibilities. This is bunch of relative complexity to take on in order to, in my case, inject our old friendly viewport meta below:

<meta
  name="viewport"
  content="minimum-scale=1, initial-scale=1, width=device-width"
/>

Instead I added support for a backwards compatible meta option.

  • when omitted the current utf-8 charset meta will be inserted.
  • when provided the charset meta must be supplied manually. To me this was the simplest implementation and an easy to doc/grok all or nothing approach.

Running tests locally some snapshots are failing with null vs undefined values for map. I'm running 12.15.0 using nvm. Perhaps there's a behavior changes between 12 and 12.xx. Tests passed for changed code, so, here we go. 🎉

Copy link
Contributor Author

@mseeley mseeley left a comment

Choose a reason for hiding this comment

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

Pre-review complete. Nothing mysterious here. :)

Default: `[{ charset: 'utf-8' }]`

Specifies attributes used to create `meta` elements. For each array member, provide an object with key-value pairs that represent meta element attribute names and values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm happy to copy/paste any documentation suggestions.

@mseeley
Copy link
Contributor Author

mseeley commented Apr 14, 2020

Resolved snapshot updating. The addition of map: undefined and map: null are still unknown.

Ah, I'm seeing CI fail with similar problems I had locally. ~I don't believe my changes are causing those failures.~ The `map` failures don't seem related. But, the snapshot failure is.

Running: pnpm run test --filter ./packages/html from the root doesn't give me the chance to update snapshots: https://gist.github.com/mseeley/44b1488096802aef9cff9dc4e974d4c2. Also, notice the map problems.

Installing AVA locally then running tests in the package does nothing when entering u to update the specs.

image

Thanks for any help sorting out what the heck is going on.

@shellscape
Copy link
Collaborator

Thanks for opening this PR. We'll get to reviewing this soon!

This allows for convenient creation of `meta` elements without reimplementing all template function responsibilities.
@mseeley
Copy link
Contributor Author

mseeley commented Apr 21, 2020

@shellscape thanks for updating the html snapshots in c09509f. Diff on this PR is now super tight. 😄

@mseeley
Copy link
Contributor Author

mseeley commented Apr 26, 2020

👋 hi maintainers. Please let me know if there's anything I can do to raise visibility of this PR. I'm hoping you all find it to be a reasonable feature addition. Thanks!

packages/html/README.md Outdated Show resolved Hide resolved
packages/html/README.md Outdated Show resolved Hide resolved
@shellscape
Copy link
Collaborator

thanks!

@shellscape shellscape merged commit 05ef80b into rollup:master Apr 26, 2020
@shellscape
Copy link
Collaborator

@mseeley If you're interested in helping to maintain the repo in the future, please let me know. We're always looking for help.

LarsDenBakker pushed a commit to LarsDenBakker/plugins that referenced this pull request Sep 12, 2020
* feat(html): support custom meta element creation

This allows for convenient creation of `meta` elements without reimplementing all template function responsibilities.

* chore: readme changes

Co-authored-by: Andrew Powell <shellscape@users.noreply.github.com>
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.

2 participants