diff options
author | Robert Speicher <rspeicher@gmail.com> | 2016-02-16 22:09:00 -0500 |
---|---|---|
committer | Robert Speicher <rspeicher@gmail.com> | 2016-02-16 22:09:00 -0500 |
commit | 6053ad847400443e8ab870cc6a4bcb14be236fb3 (patch) | |
tree | 67e0a3a0b3c69208d2925e5dc896b54c77083787 /doc/development/gotchas.md | |
parent | e82b17fa43822d765cad1b813227032b267b3781 (diff) | |
download | gitlab-ce-6053ad847400443e8ab870cc6a4bcb14be236fb3.tar.gz |
Add "Gotchas" development doc
Diffstat (limited to 'doc/development/gotchas.md')
-rw-r--r-- | doc/development/gotchas.md | 104 |
1 files changed, 104 insertions, 0 deletions
diff --git a/doc/development/gotchas.md b/doc/development/gotchas.md new file mode 100644 index 00000000000..d2d8a6119dd --- /dev/null +++ b/doc/development/gotchas.md @@ -0,0 +1,104 @@ +# Gotchas + +The purpose of this guide is to document potential "gotchas" that contributors +might encounter or should avoid during development of GitLab CE and EE. + +## Don't `describe` symbols + +Consider the following model spec: + +```ruby +require 'rails_helper' + +describe User do + describe :to_param do + it 'converts the username to a param' do + user = described_class.new(username: 'John Smith') + + expect(user.to_param).to eq 'john-smith' + end + end +end +``` + +When run, this spec doesn't do what we might expect: + +```sh +spec/models/user_spec.rb|6 error| Failure/Error: u = described_class.new NoMethodError: undefined method `new' for :to_param:Symbol +``` + +### Solution + +Except for the top-level `describe` block, always provide a String argument to +`describe`. + +## Don't `rescue Exception` + +See ["Why is it bad style to `rescue Exception => e` in Ruby?"][Exception]. + +_**Note:** This rule is [enforced automatically by +Rubocop](https://gitlab.com/gitlab-org/gitlab-ce/blob/8-4-stable/.rubocop.yml#L911-914)._ + +[Exception]: http://stackoverflow.com/q/10048173/223897 + +## Don't use inline CoffeeScript in views + +Using the inline `:coffee` or `:coffeescript` Haml filters comes with a +performance overhead. + +We've [removed these two filters entirely](https://gitlab.com/gitlab-org/gitlab-ce/blob/8-5-stable/config/initializers/haml.rb) +through an initializer. + +### Further reading + +- Pull Request: [Replace CoffeeScript block into JavaScript in Views](https://git.io/vztMu) +- Stack Overflow: [Performance implications of using :coffescript filter inside HAML templates?](http://stackoverflow.com/a/17571242/223897) + +## ID-based CSS selectors need to be a bit more specific + +Normally, because HTML `id` attributes need to be unique to the page, it's +perfectly fine to write some JavaScript like the following: + +```javascript +$('#js-my-selector').hide(); +``` + +But there's a feature of GitLab's Markdown processing that will automatically +add `id` attributes underneath header elements in order to make them linkable. +The content of the header is ["dasherized"][ToC Processing] and used in the `id` +attribute. + +Unfortunately, this feature makes it possible for user-generated content to +create a header element with the same `id` attribute we're using in our +selector, potentially breaking the JavaScript behavior. A user could break the +above example JavaScript with the following Markdown: + +```markdown +## JS My Selector +``` + +Which gets converted to the following HTML after processing: + +```html +<h2> + <a id="js-my-selector" class="anchor" href="#js-my-selector" aria-hidden="true"></a> + JS My Selector +</h2> +``` + +[ToC Processing]: https://gitlab.com/gitlab-org/gitlab-ce/blob/8-4-stable/lib/banzai/filter/table_of_contents_filter.rb#L31-37 + +### Solution + +The current recommended fix for this is to make our selectors slightly more +specific: + +```javascript +$('div#js-my-selector').hide(); +``` + +### Further reading + +- Issue: [Merge request ToC anchor conflicts with tabs](https://gitlab.com/gitlab-org/gitlab-ce/issues/3908) +- Merge Request: [Make tab target selectors less naive](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/2023) +- Merge Request: [Make cross-project reference's clipboard target less naive](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/2024) |